Ne vous perdez pas dans trois si. Refactoriser les conditions de branchement

Sur Internet, vous pouvez trouver de nombreuses descriptions de techniques pour simplifier les expressions conditionnelles (par exemple, ici ). Dans ma pratique, j'utilise parfois une combinaison de substitution d'opérateur de frontière de conditionnelles imbriquées et de concaténation conditionnelle . Cela donne généralement un bon résultat lorsque le nombre de conditions indépendantes et d'expressions exécutées est sensiblement inférieur au nombre de branches dans lesquelles elles sont combinées de différentes manières. Le code sera en C #, mais les étapes sont les mêmes pour tout langage prenant en charge les constructions if / else.



image



Donné



Il existe une interface IUnit .



IUnit
public interface IUnit
{
    string Description { get; }
}


Et ses implémentations Piece et Cluster .



Pièce
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;
        }
    }
}


Grappe
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;
        }
    }
}


Il existe également une classe MergeClusters qui gère les collections IUnit et fusionne les séquences de cluster compatibles en un seul élément. Le comportement de la classe est vérifié par des tests.



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);
}


Nous nous intéressons à la méthode 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);
        }
    }
}


D'une part, cela fonctionne correctement, mais d'autre part, je voudrais le rendre plus expressif et, si possible, améliorer les métriques du code. Nous calculerons les métriques à l'aide de l'outil Analyser> Calculer les métriques de code , qui fait partie de la communauté Visual Studio . Au départ, ils signifient:



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 général, l'approche décrite ci-dessous ne garantit pas un beau résultat.



Blague barbu pour l'occasion
#392487

. , . , .

© bash.org



Refactoring



Étape 1



Nous vérifions que chaque chaîne de conditions du même niveau d'imbrication se termine par un bloc else , sinon nous ajoutons un bloc else vide .



Résultat
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);
        }
    }
}


Étape 2



Si les expressions existent au même niveau d'imbrication que les blocs conditionnels, nous enveloppons chaque expression dans chaque bloc conditionnel. Si l'expression précède le bloc, ajoutez-la au début du bloc, sinon à la fin. Nous répétons jusqu'à ce que, à chaque niveau d'imbrication, les blocs conditionnels soient adjacents uniquement aux autres blocs conditionnels.



Résultat
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);
        }
    }
}


Étape 3



À chaque niveau d'imbrication, pour chaque bloc if , nous coupons le reste de la chaîne de conditions, créons un nouveau bloc if avec l'expression opposée à l'expression du premier if , plaçons la chaîne de coupe dans le nouveau bloc et supprimons le premier mot else . Répétez jusqu'à ce qu'il ne reste plus rien d' autre .



Résultat
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);
                }
            }
        }
    }
}


Étape 4



Nous «effondrons» les blocs.



Résultat
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);
        }
    }
}


Étape 5



Aux conditions de chaque bloc if qui n'a pas de blocs imbriqués, utilisez l'opérateur && pour ajouter les conditions de tous les blocs if parents .



Résultat
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);
        }
    }
}


Étape 6



Nous laissons seulement si des blocs qui ne sont pas des blocs imbriqués, en gardant l'ordre de leur apparition dans le code.



Résultat
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);
    }
}


Étape 7



Pour chaque expression unique, dans l'ordre de leur apparition dans le code, nous écrivons les blocs les contenant. En même temps, nous ignorons les autres expressions à l'intérieur des blocs.



Résultat
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);
    }
}


Étape 8



Nous combinons des blocs avec les mêmes expressions en appliquant l'opérateur || à leurs conditions. ...

Résultat
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);
    }
}


Étape 9



Simplification des expressions conditionnelles à l'aide des règles de l'algèbre booléenne .



Résultat
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);
    }
}


Étape 10



Nous nous redressons avec un fichier.



Résultat
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



Après la refactorisation, la méthode ressemble à ceci:



private void MergeNeighbors(IUnit prev, IUnit next)
{
    if (IsEndOfCompatibleClusterSequence(prev, next))
        Flush();

    if (next is Cluster)
        buffer.Add((Cluster)next);
    else
        merged.Add(next);
}


Et les métriques sont comme ceci:



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


Les métriques se sont considérablement améliorées et le code est devenu beaucoup plus court et plus expressif. Mais la chose la plus remarquable à propos de cette approche, pour moi personnellement, est la suivante: quelqu'un est capable de voir immédiatement que la méthode devrait ressembler à la version finale, et quelqu'un ne peut écrire que l'implémentation initiale, mais ayant au moins une formulation comportement correct, à l'aide d'actions purement mécaniques (à l'exception peut-être de la dernière étape), il peut être amené à la forme la plus concise et visuelle.



PS Toutes les connaissances développées dans l'algorithme décrit dans la publication ont été obtenues par l'auteur à l'école il y a plus de 15 ans. Pour lequel il exprime sa profonde gratitude aux enseignants enthousiastes qui donnent aux enfants la base d'une éducation normale. Tatyana Alekseevna, Natalya Pavlovna, si vous lisez soudainement ceci, merci beaucoup!



All Articles