No se pierda en tres si. Refactorización de condiciones de ramificación

En Internet, puede encontrar muchas descripciones de técnicas para simplificar expresiones condicionales (por ejemplo, aquí ). En mi práctica, a veces uso una combinación de sustitución de operadores de límite de condicionales anidados y concatenación condicional . Por lo general, da un buen resultado cuando el número de condiciones independientes y expresiones ejecutadas es notablemente menor que el número de ramas en las que se combinan de diferentes formas. El código estará en C #, pero los pasos son los mismos para cualquier lenguaje que admita construcciones if / else.



imagen



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



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!



All Articles