Dado
Hay una interfaz IUnit .
IUnit
public interface IUnit
{
string Description { get; }
}
Y sus implementaciones Piece y Cluster .
Pedazo
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;
}
}
}
Racimo
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;
}
}
}
También hay una clase MergeClusters que maneja colecciones IUnit y fusiona secuencias Cluster compatibles en un solo elemento. El comportamiento de la clase se verifica mediante pruebas.
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);
}
Estamos interesados en el método void MergeNeighbors (IUnit, IUnit) class MergeClusters .
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);
}
}
}
Por un lado, funciona correctamente, pero por otro lado, me gustaría hacerlo más expresivo y, si es posible, mejorar las métricas del código. Calcularemos las métricas usando la herramienta Analizar> Calcular métricas de código , que es parte de la comunidad de Visual Studio . Inicialmente, quieren decir:
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
En general, el enfoque que se describe a continuación no garantiza un resultado hermoso.
Chiste barbudo para la ocasión
#392487
. , . , .
© bash.org
. , . , .
© bash.org
Refactorización
Paso 1
Verificamos que cada cadena de condiciones del mismo nivel de anidamiento termina con un bloque else , de lo contrario agregamos un bloque else vacío .
Resultado
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);
}
}
}
Paso 2
Si hay expresiones en el mismo nivel de anidamiento con bloques condicionales, ajustamos cada expresión a cada bloque condicional. Si la expresión precede al bloque, la agregamos al principio del bloque; de lo contrario, al final. Repetimos hasta que, en cada nivel de anidación, los bloques condicionales son adyacentes solo a otros bloques condicionales.
Resultado
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);
}
}
}
Paso 3
En cada nivel de anidamiento, para cada bloque if , cortamos el resto de la cadena de condiciones, creamos un nuevo bloque if con la expresión opuesta a la expresión del primer if , colocamos la cadena cortada dentro del nuevo bloque y eliminamos la primera palabra else . Repita hasta que no quede nada más .
Resultado
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);
}
}
}
}
}
Paso 4
"Colapsamos" los bloques.
Resultado
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);
}
}
}
Paso 5
A las condiciones de cada bloque if que no tiene bloques anidados, use el operador && para agregar las condiciones de todos los bloques if padres .
Resultado
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);
}
}
}
Paso 6
Dejamos solo si son bloques que no tienen bloques anidados, manteniendo el orden de su aparición en el código.
Resultado
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);
}
}
Paso 7
Para cada expresión única, en el orden en que aparecen en el código, escribimos los bloques que las contienen. Al mismo tiempo, ignoramos otras expresiones dentro de los bloques.
Resultado
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);
}
}
Paso 8
Combinamos bloques con las mismas expresiones aplicando el operador || a sus condiciones. ...
Resultado
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);
}
}
Paso 9
Simplifique las expresiones condicionales usando las reglas del álgebra booleana .
Resultado
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);
}
}
Paso 10
Enderezamos con una lima.
Resultado
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));
Total
Después de la refactorización, el método se ve así:
private void MergeNeighbors(IUnit prev, IUnit next)
{
if (IsEndOfCompatibleClusterSequence(prev, next))
Flush();
if (next is Cluster)
buffer.Add((Cluster)next);
else
merged.Add(next);
}
Y las métricas son así:
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
Las métricas han mejorado significativamente y el código se ha vuelto mucho más corto y expresivo. Pero lo más notable de este enfoque, para mí personalmente, es esto: alguien puede ver inmediatamente que el método debería verse como en la versión final, y alguien puede escribir solo la implementación inicial, pero con al menos alguna formulación comportamiento correcto, con la ayuda de acciones puramente mecánicas (excepto, quizás, el último paso), se puede llevar a la forma más lacónica y visual.
PD Todos los conocimientos que se han desarrollado en el algoritmo descrito en la publicación fueron obtenidos por el autor en la escuela hace más de 15 años. Por lo cual expresa su profundo agradecimiento a los entusiastas maestros que dan a los niños la base de una educación normal. Tatyana Alekseevna, Natalya Pavlovna, si de repente estás leyendo esto, ¡muchas gracias!