Не заблудиться в трёх if'ах. Рефакторинг ветвящихся условий
На просторах интернета можно найти множество описаний приемов упрощения условных выражений (например, тут). В своей практике я иногда использую комбинацию замены вложенных условных операторов граничным оператором и объединения условных операторов. Обычно она дает красивый результат, когда количество независимых условий и выполняемых выражений заметно меньше количества веток, в которых они комбинируются различными способами. Код будет на C#, но действия одинаковы для любого языка, поддерживающего конструкции if/else.
Дано
Есть интерфейс IUnit.
public interface IUnit
{
string Description { get; }
}
И его реализации Piece и Cluster.
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;
}
}
}
public class Cluster : IUnit
{
private readonly IReadOnlyList pieces;
public IEnumerable Pieces => pieces;
public string Description { get; }
public Cluster(IEnumerable 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 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 в один элемент. Поведение класса проверяется тестами.
public class MergeClusters
{
private readonly List buffer = new List();
private List merged;
private readonly IReadOnlyList units;
public IEnumerable Result
{
get
{
if (merged != null)
return merged;
merged = new List();
Merge();
return merged;
}
}
public MergeClusters(IEnumerable 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();
}
}
[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 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 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 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 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 лет назад. За что он выражает огромную благодарность учителям-энтузиастам, дающим детям основу нормального образования. Татьяна Алексеевна, Наталья Павловна, если вы вдруг это читаете, большое вам СПАСИБО!
