◼️ 👨🏿 ⚰️ Don't get lost in three if's. Refactoring branching conditions 🧡 ⚗️ 🏂🏼

On the Internet, you can find many descriptions of techniques for simplifying conditional expressions (for example, here ). In my practice, I sometimes use a combination of boundary operator substitution of nested conditionals and conditional concatenation . It usually gives a nice result when the number of independent conditions and executed expressions is noticeably less than the number of branches in which they are combined in different ways. The code will be in C #, but the steps are the same for any language that supports if / else constructs.

Given

There is an IUnit interface .

IUnit

public interface IUnit
{
    string Description { get; }
}

And its implementations Piece and Cluster .

Piece

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

There is also a MergeClusters class that handles IUnit collections and merges compatible Cluster sequences into a single item. The behavior of the class is verified by 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);
}

We are interested in the method 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);
        }
    }
}

On the one hand, it works correctly, but on the other hand, I would like to make it more expressive and, if possible, improve the code metrics. We will calculate the metrics using the Analyze> Calculate Code Metrics tool , which is part of the Visual Studio Community . Initially, they mean:

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

In general, the approach described below does not guarantee a beautiful result.

Bearded joke for the occasion

#392487

. , . , .

© bash.org

Refactoring

Step 1

We check that each chain of conditions of the same nesting level ends with an else block , otherwise we add an empty else block .

Result

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

Step 2

If expressions exist at the same nesting level as conditional blocks, we wrap each expression to each conditional block. If the expression precedes the block, we add it to the beginning of the block; otherwise, to the end. We repeat until, at each nesting level, conditional blocks are adjacent only to other conditional blocks.

Result

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

Step 3

At each nesting level, for each if block , we cut off the rest of the chain of conditions, create a new if block with the expression opposite to the expression of the first if , put the cut chain inside the new block and delete the first word else . Repeat until there are no else left .

Result

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

Step 4

We "collapse" the blocks.

Result

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

Step 5

To the conditions of each if block that does not have nested blocks, use the && operator to add the conditions of all parent if blocks .

Result

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

Step 6

We leave only if blocks that do not have nested blocks, keeping the order of their appearance in the code.

Result

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

Step 7

For each unique expression, in the order of their appearance in the code, we write out the blocks containing them. At the same time, we ignore other expressions inside the blocks.

Result

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

Step 8

We combine blocks with the same expressions by applying the || operator to their conditions. ...

Result

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

Step 9

Simplify conditional expressions using the rules of Boolean algebra .

Result

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

Step 10

We straighten with a file.

Result

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

After refactoring, the method looks like this:

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

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

And the metrics are like this:

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

Metrics have improved significantly, and the code has become much shorter and more expressive. But the most remarkable thing about this approach, for me personally, is this: someone is able to immediately see that the method should look like in the final version, and someone can write only the initial implementation, but having at least some formulation correct behavior, with the help of purely mechanical actions (with the exception, perhaps, of the last step), it can be brought to the most concise and visual form.

PS All the pieces of knowledge that have developed into the algorithm described in the publication were obtained by the author at school more than 15 years ago. For which he expresses his deep gratitude to the enthusiastic teachers who give children the foundation of a normal education. Tatyana Alekseevna, Natalya Pavlovna, if you are suddenly reading this, thank you very much!

Don't get lost in three if's. Refactoring branching conditions

Given

Refactoring

Step 1

Step 2

Step 3

Step 4

Step 5

Step 6

Step 7

Step 8

Step 9

Step 10

Total

More articles: