Не заблудиться в трёх if'ах. Рефакторинг ветвящихся условий

Моя цель - предложение широкого ассортимента товаров и услуг на постоянно высоком качестве обслуживания по самым выгодным ценам.

Прежде чем перейти к статье, хочу вам представить, экономическую онлайн игру Brave Knights, в которой вы можете играть и зарабатывать. Регистируйтесь, играйте и зарабатывайте!

На просторах интернета можно найти множество описаний приемов упрощения условных выражений (например, тут). В своей практике я иногда использую комбинацию замены вложенных условных операторов граничным оператором и объединения условных операторов. Обычно она дает красивый результат, когда количество независимых условий и выполняемых выражений заметно меньше количества веток, в которых они комбинируются различными способами. Код будет на C#, но действия одинаковы для любого языка, поддерживающего конструкции if/else.

image

Дано


Есть интерфейс IUnit.

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

И его реализации Piece и 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;
        }
    }
}

Также есть класс MergeClusters, который обрабатывает коллекции IUnit и объединяет последовательности совместимых Cluster в один элемент. Поведение класса проверяется тестами.

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

Нас интересует метод void MergeNeighbors(IUnit, IUnit) класса 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);
        }
    }
}

С одной стороны, он работает правильно, но с другой, хотелось бы сделать его более выразительным и по возможности улучшить метрики кода. Метрики будем считать с помощью инструмента Analyze > Calculate Code Metrics, который входит в состав Visual Studio Community. Изначально они имеют значения:

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

Описанный далее подход в общем случае не гарантирует красивый результат.

Бородатая шутка по случаю
#392487
Мне недавно рассказали как делают корабли в бутылках. В бутылку засыпают силикатного клея, говна и трясут. Получаются разные странные штуки, иногда корабли.
© bash.org

Рефакторинг


Шаг 1


Проверяем, что каждая цепочка условий одного уровня вложенности заканчивается блоком else, в противном случае добавляем пустой блок else.

Результат 
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);
        }
    }
}

Шаг 2


Если на одном уровне вложенности с условными блоками существуют выражения, переносим каждое выражение к каждый условный блок. Если выражение предшествует блоку, добавляем его в начало блока, в противном случае — в конец. Повторяем, пока на каждом уровне вложенности условные блоки не будут соседствовать только с другими условными блоками.

Результат 
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);
        }
    }
}

Шаг 3


На каждом уровне вложенности для каждого блока if отрезаем остаток цепочки условий, создаем новый блок if с выражением, обратным выражению первого if, помещаем внутрь нового блока вырезанную цепочку и удаляем первое слово else. Повторяем, пока не останется ни одного else.

Результат 
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);
                }
            }
        }
    }
}

Шаг 4


«Схлопываем» блоки.

Результат 
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);
        }
    }
}

Шаг 5


К условиям каждого блока if, не имеющего вложенных блоков, с помощью оператора && добавляем условия всех родительский блоков if.

Результат 
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);
        }
    }
}

Шаг 6


Оставляем только блоки if, не имеющие вложенных блоков, сохраняя порядок их появления в коде.

Результат 
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);
    }
}

Шаг 7


Для каждого уникального выражения в порядке их появления в коде выписываем содержащие их блоки. При этом другие выражения внутри блоков игнорируем.

Результат 
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);
    }
}

Шаг 8


Объединяем блоки с одинаковыми выражениями, применяя к их условиям оператор ||.
Результат 
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);
    }
}

Шаг 9


Упрощаем условные выражения с помощью правил булевой алгебры.

Результат 
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);
    }
}

Шаг 10


Рихтуем напильником.

Результат 
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));

Итого


После рефакторинга метод выглядит так:

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

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

А метрики так:

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

Метрики заметно улучшились, а код стал гораздо короче и выразительнее. Но самым замечательным в этом подходе, лично для меня, является вот что: кто-то способен сразу увидеть, что метод должен выглядеть, как в конечном варианте, а кто-то может написать только изначальную реализацию, но имея хотя бы какую-то формулировку правильного поведения, с помощью чисто механических действий (за исключением, возможно, последнего шага) ее можно привести к наиболее лаконичному и наглядному виду.

P.S. Все куски знаний, которые сложились в описанный в публикации алгоритм, были получены автором еще в школе более 15 лет назад. За что он выражает огромную благодарность учителям-энтузиастам, дающим детям основу нормального образования. Татьяна Алексеевна, Наталья Павловна, если вы вдруг это читаете, большое вам СПАСИБО!
Источник: https://habr.com/ru/post/516286/

Вернуться к списку

Интересные статьи

Интересные статьи

Go в продуктовой разработке: посмотрим с трех сторон
Go в продуктовой разработке: посмотрим с трех сторон
До Golang Live 2020 уже рукой подать. С 14 по 17 октября мы встречаемся на платформе Онтико. И даже в онлайн-формате, помимо традиционных докладов, будут интерактивные форматы: воркшо...
Фракталы в песках, или Больше трёх не собираться
Фракталы в песках, или Больше трёх не собираться
Мы поговорим о модели песчаной кучи. Песок (не настоящий, модельный), пересыпаясь, создаёт вот такие картинки: Песчаные кучи можно складывать (это легко, если вы привыкли складывать всякие...
Поэтапное внедрение фишек в интернет-магазине 1C-Битрикс
Поэтапное внедрение фишек в интернет-магазине 1C-Битрикс
Существует традиция, долго и дорого разрабатывать интернет-магазин. :-) Лакировать все детали, придумывать, внедрять и полировать «фишечки» и делать это все до открытия магазина.
Внедрение CRM Битрикс24 в большой компании
Внедрение CRM Битрикс24 в большой компании
Компании растут и меняются. Если для небольшого бизнеса легко прогнозировать последствия любых изменений, то у крупного для такого предвидения — необходимо изучение деталей.
SMS-мониторинг веса трех ульев за 35$
Прошло совсем немного времени с предыдущей публикации о системе за 30$. Что-же изменилось за прошедший лунный месяц? Добавлена солнечная батарейка — время работы практически неограниченно. ...