Gegeben
Es gibt eine IUnit- Schnittstelle .
IUnit
public interface IUnit
{
string Description { get; }
}
Und seine Implementierungen Piece und Cluster .
Stück
public class Piece : IUnit
{
public string Description { get; }
public Piece(string description) =>
Description = description;
public override bool Equals(object obj) =>
Equals(obj as Piece);
public bool Equals(Piece piece) =>
piece != null &&
piece.Description.Equals(Description);
public override int GetHashCode()
{
unchecked
{
var hash = 17;
foreach (var c in Description)
hash = 23 * hash + c.GetHashCode();
return hash;
}
}
}
Cluster
public class Cluster : IUnit
{
private readonly IReadOnlyList<Piece> pieces;
public IEnumerable<Piece> Pieces => pieces;
public string Description { get; }
public Cluster(IEnumerable<Piece> pieces)
{
if (!pieces.Any())
throw new ArgumentException();
if (pieces.Select(unit => unit.Description).Distinct().Count() > 1)
throw new ArgumentException();
this.pieces = pieces.ToArray();
Description = this.pieces[0].Description;
}
public Cluster(IEnumerable<Cluster> clusters)
: this(clusters.SelectMany(cluster => cluster.Pieces))
{
}
public override bool Equals(object obj) =>
Equals(obj as Cluster);
public bool Equals(Cluster cluster) =>
cluster != null &&
cluster.Description.Equals(Description) &&
cluster.pieces.Count == pieces.Count;
public override int GetHashCode()
{
unchecked
{
var hash = 17;
foreach (var c in Description)
hash = 23 * hash + c.GetHashCode();
hash = 23 * hash + pieces.Count.GetHashCode();
return hash;
}
}
}
Es gibt auch eine MergeClusters- Klasse , die IUnit- Sammlungen verarbeitet und kompatible Cluster- Sequenzen zu einem einzigen Element zusammenführt. Das Verhalten der Klasse wird durch Tests überprüft.
MergeClusters
public class MergeClusters
{
private readonly List<Cluster> buffer = new List<Cluster>();
private List<IUnit> merged;
private readonly IReadOnlyList<IUnit> units;
public IEnumerable<IUnit> Result
{
get
{
if (merged != null)
return merged;
merged = new List<IUnit>();
Merge();
return merged;
}
}
public MergeClusters(IEnumerable<IUnit> units)
{
this.units = units.ToArray();
}
private void Merge()
{
Seed();
for (var i = 1; i < units.Count; i++)
MergeNeighbors(units[i - 1], units[i]);
Flush();
}
private void Seed()
{
if (units[0] is Cluster)
buffer.Add((Cluster)units[0]);
else
merged.Add(units[0]);
}
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (prev is Cluster)
{
if (next is Cluster)
{
if (!prev.Description.Equals(next.Description))
{
Flush();
}
buffer.Add((Cluster)next);
}
else
{
Flush();
merged.Add(next);
}
}
else
{
if (next is Cluster)
{
buffer.Add((Cluster)next);
}
else
{
merged.Add(next);
}
}
}
private void Flush()
{
if (!buffer.Any())
return;
merged.Add(new Cluster(buffer));
buffer.Clear();
}
}
MergeClustersTests
[Fact]
public void Result_WhenUnitsStartWithNonclusterAndEndWithCluster_IsCorrect()
{
// Arrange
IUnit[] units = new IUnit[]
{
new Piece("some description"),
new Piece("some description"),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
};
MergeClusters sut = new MergeClusters(units);
// Act
IEnumerable<IUnit> actual = sut.Result;
// Assert
IUnit[] expected = new IUnit[]
{
new Piece("some description"),
new Piece("some description"),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
};
actual.Should().BeEquivalentTo(expected);
}
[Fact]
public void Result_WhenUnitsStartWithClusterAndEndWithCluster_IsCorrect()
{
// Arrange
IUnit[] units = new IUnit[]
{
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
};
MergeClusters sut = new MergeClusters(units);
// Act
IEnumerable<IUnit> actual = sut.Result;
// Assert
IUnit[] expected = new IUnit[]
{
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
};
actual.Should().BeEquivalentTo(expected);
}
[Fact]
public void Result_WhenUnitsStartWithClusterAndEndWithNoncluster_IsCorrect()
{
// Arrange
IUnit[] units = new IUnit[]
{
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
};
MergeClusters sut = new MergeClusters(units);
// Act
IEnumerable<IUnit> actual = sut.Result;
// Assert
IUnit[] expected = new IUnit[]
{
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
};
actual.Should().BeEquivalentTo(expected);
}
[Fact]
public void Result_WhenUnitsStartWithNonclusterAndEndWithNoncluster_IsCorrect()
{
// Arrange
IUnit[] units = new IUnit[]
{
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
};
MergeClusters sut = new MergeClusters(units);
// Act
IEnumerable<IUnit> actual = sut.Result;
// Assert
IUnit[] expected = new IUnit[]
{
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("some description"),
new Piece("some description"),
new Piece("some description"),
new Piece("some description"),
}),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
new Cluster(
new Piece[]
{
new Piece("another description"),
new Piece("another description"),
new Piece("another description"),
new Piece("another description"),
}),
new Piece("another description"),
};
actual.Should().BeEquivalentTo(expected);
}
Wir sind an der Methode void MergeNeighbors (IUnit, IUnit) der MergeClusters- Klasse interessiert .
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (prev is Cluster)
{
if (next is Cluster)
{
if (!prev.Description.Equals(next.Description))
{
Flush();
}
buffer.Add((Cluster)next);
}
else
{
Flush();
merged.Add(next);
}
}
else
{
if (next is Cluster)
{
buffer.Add((Cluster)next);
}
else
{
merged.Add(next);
}
}
}
Einerseits funktioniert es korrekt, andererseits möchte ich es ausdrucksvoller machen und, wenn möglich, die Codemetriken verbessern. Wir berechnen die Metriken mit dem Tool Analysieren> Code-Metriken berechnen , das Teil der Visual Studio-Community ist . Anfangs bedeuten sie:
Configuration: Debug
Member: MergeNeighbors(IUnit, IUnit) : void
Maintainability Index: 64
Cyclomatic Complexity: 5
Class Coupling: 4
Lines of Source code: 32
Lines of Executable code: 10
Im Allgemeinen garantiert der unten beschriebene Ansatz kein schönes Ergebnis.
Bartwitz bei dieser Gelegenheit
#392487
. , . , .
© bash.org
. , . , .
© bash.org
Refactoring
Schritt 1
Wir überprüfen, ob jede Kette von Bedingungen derselben Verschachtelungsebene mit einem else- Block endet , andernfalls fügen wir einen leeren else- Block hinzu .
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (prev is Cluster)
{
if (next is Cluster)
{
if (!prev.Description.Equals(next.Description))
{
Flush();
}
else
{
}
buffer.Add((Cluster)next);
}
else
{
Flush();
merged.Add(next);
}
}
else
{
if (next is Cluster)
{
buffer.Add((Cluster)next);
}
else
{
merged.Add(next);
}
}
}
Schritt 2
Wenn es Ausdrücke auf derselben Verschachtelungsebene mit bedingten Blöcken gibt, wird jeder Ausdruck in jeden bedingten Block eingeschlossen. Wenn der Ausdruck vor dem Block steht, fügen wir ihn am Anfang des Blocks hinzu, andernfalls am Ende. Wir wiederholen, bis auf jeder Verschachtelungsebene bedingte Blöcke nur noch an andere bedingte Blöcke angrenzen.
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (prev is Cluster)
{
if (next is Cluster)
{
if (!prev.Description.Equals(next.Description))
{
Flush();
buffer.Add((Cluster)next);
}
else
{
buffer.Add((Cluster)next);
}
}
else
{
Flush();
merged.Add(next);
}
}
else
{
if (next is Cluster)
{
buffer.Add((Cluster)next);
}
else
{
merged.Add(next);
}
}
}
Schritt 3
Auf jeder Verschachtelungsebene schneiden wir für jeden if- Block den Rest der Bedingungskette ab, erstellen einen neuen if- Block mit dem Ausdruck, der dem Ausdruck des ersten if entgegengesetzt ist , setzen die geschnittene Kette in den neuen Block und löschen das erste andere Wort . Wiederholen, bis nichts mehr übrig ist .
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (prev is Cluster)
{
if (next is Cluster)
{
if (!prev.Description.Equals(next.Description))
{
Flush();
buffer.Add((Cluster)next);
}
if (prev.Description.Equals(next.Description))
{
{
buffer.Add((Cluster)next);
}
}
}
if (!(next is Cluster))
{
{
Flush();
merged.Add(next);
}
}
}
if (!(prev is Cluster))
{
{
if (next is Cluster)
{
buffer.Add((Cluster)next);
}
if (!(next is Cluster))
{
{
merged.Add(next);
}
}
}
}
}
Schritt 4
Wir "kollabieren" die Blöcke.
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (prev is Cluster)
{
if (next is Cluster)
{
if (!prev.Description.Equals(next.Description))
{
Flush();
buffer.Add((Cluster)next);
}
if (prev.Description.Equals(next.Description))
{
buffer.Add((Cluster)next);
}
}
if (!(next is Cluster))
{
Flush();
merged.Add(next);
}
}
if (!(prev is Cluster))
{
if (next is Cluster)
{
buffer.Add((Cluster)next);
}
if (!(next is Cluster))
{
merged.Add(next);
}
}
}
Schritt 5
Verwenden Sie zu den Bedingungen jedes if- Blocks , der keine verschachtelten Blöcke enthält, den Operator && , um die Bedingungen aller übergeordneten if- Blöcke hinzuzufügen .
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (prev is Cluster)
{
if (next is Cluster)
{
if (!prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster)
{
Flush();
buffer.Add((Cluster)next);
}
if (prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster)
{
buffer.Add((Cluster)next);
}
}
if (!(next is Cluster) && prev is Cluster)
{
Flush();
merged.Add(next);
}
}
if (!(prev is Cluster))
{
if (next is Cluster && !(prev is Cluster))
{
buffer.Add((Cluster)next);
}
if (!(next is Cluster) && !(prev is Cluster))
{
merged.Add(next);
}
}
}
Schritt 6
Wir lassen nur , wenn Blöcke , die verschachtelte Blöcke nicht haben, in den Code der Reihenfolge ihres Erscheinens zu bewahren.
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (!prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster)
{
Flush();
buffer.Add((Cluster)next);
}
if (prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster)
{
buffer.Add((Cluster)next);
}
if (!(next is Cluster) && prev is Cluster)
{
Flush();
merged.Add(next);
}
if (next is Cluster && !(prev is Cluster))
{
buffer.Add((Cluster)next);
}
if (!(next is Cluster) && !(prev is Cluster))
{
merged.Add(next);
}
}
Schritt 7
Für jeden eindeutigen Ausdruck schreiben wir in der Reihenfolge ihres Auftretens im Code die Blöcke aus, die sie enthalten. Gleichzeitig ignorieren wir andere Ausdrücke innerhalb der Blöcke.
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (!prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster)
{
Flush();
}
if (!(next is Cluster) && prev is Cluster)
{
Flush();
}
if (!prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster)
{
buffer.Add((Cluster)next);
}
if (prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster)
{
buffer.Add((Cluster)next);
}
if (next is Cluster && !(prev is Cluster))
{
buffer.Add((Cluster)next);
}
if (!(next is Cluster) && prev is Cluster)
{
merged.Add(next);
}
if (!(next is Cluster) && !(prev is Cluster))
{
merged.Add(next);
}
}
Schritt 8
Wir kombinieren Blöcke mit denselben Ausdrücken, indem wir den Operator || auf ihre Bedingungen anwenden. ...
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (!prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster ||
!(next is Cluster) && prev is Cluster)
{
Flush();
}
if (!prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster ||
prev.Description.Equals(next.Description) && next is Cluster && prev is Cluster ||
next is Cluster && !(prev is Cluster))
{
buffer.Add((Cluster)next);
}
if (!(next is Cluster) && prev is Cluster ||
!(next is Cluster) && !(prev is Cluster))
{
merged.Add(next);
}
}
Schritt 9
Vereinfachen Sie bedingte Ausdrücke nach den Regeln der Booleschen Algebra .
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (prev is Cluster && !(next is Cluster && prev.Description.Equals(next.Description)))
{
Flush();
}
if (next is Cluster)
{
buffer.Add((Cluster)next);
}
if (!(next is Cluster))
{
merged.Add(next);
}
}
Schritt 10
Wir richten uns mit einer Feile auf.
Ergebnis
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (IsEndOfCompatibleClusterSequence(prev, next))
Flush();
if (next is Cluster)
buffer.Add((Cluster)next);
else
merged.Add(next);
}
private static bool IsEndOfCompatibleClusterSequence(IUnit prev, IUnit next) =>
prev is Cluster && !(next is Cluster && prev.Description.Equals(next.Description));
Gesamt
Nach dem Refactoring sieht die Methode folgendermaßen aus:
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (IsEndOfCompatibleClusterSequence(prev, next))
Flush();
if (next is Cluster)
buffer.Add((Cluster)next);
else
merged.Add(next);
}
Und die Metriken sind wie folgt:
Configuration: Debug
Member: MergeNeighbors(IUnit, IUnit) : void
Maintainability Index: 82
Cyclomatic Complexity: 3
Class Coupling: 3
Lines of Source code: 10
Lines of Executable code: 2
Die Metriken haben sich erheblich verbessert, und der Code ist viel kürzer und aussagekräftiger geworden. Das Bemerkenswerteste an diesem Ansatz ist für mich persönlich: Jemand kann sofort erkennen, dass die Methode wie in der endgültigen Version aussehen sollte, und jemand kann nur die ursprüngliche Implementierung schreiben, aber zumindest eine Formulierung haben korrektes Verhalten kann mit Hilfe rein mechanischer Handlungen (außer vielleicht dem letzten Schritt) in die lakonischste und visuellste Form gebracht werden.
PS Alle Erkenntnisse, die sich zu dem in der Veröffentlichung beschriebenen Algorithmus entwickelt haben, wurden vom Autor vor mehr als 15 Jahren in der Schule erworben. Dafür bedankt er sich bei den begeisterten Lehrern, die den Kindern die Grundlage für eine normale Ausbildung geben. Tatyana Alekseevna, Natalya Pavlovna, wenn Sie dies plötzlich lesen, vielen Dank!