Дано
Есть интерфейс 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: DebugMember: MergeNeighbors(IUnit, IUnit) : voidMaintainability Index: 64Cyclomatic Complexity: 5Class Coupling: 4Lines of Source code: 32Lines of Executable code: 10
Описанный далее подход в общем случае не гарантирует красивый результат.
Бородатая шутка по случаю
#392487
Мне недавно рассказали как делают корабли в бутылках. В бутылку засыпают силикатного клея, говна и трясут. Получаются разные странные штуки, иногда корабли.
bash.org
Мне недавно рассказали как делают корабли в бутылках. В бутылку засыпают силикатного клея, говна и трясут. Получаются разные странные штуки, иногда корабли.
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: DebugMember: MergeNeighbors(IUnit, IUnit) : voidMaintainability Index: 82Cyclomatic Complexity: 3Class Coupling: 3Lines of Source code: 10Lines of Executable code: 2
Метрики заметно улучшились, а код стал гораздо короче и выразительнее. Но самым замечательным в этом подходе, лично для меня, является вот что: кто-то способен сразу увидеть, что метод должен выглядеть, как в конечном варианте, а кто-то может написать только изначальную реализацию, но имея хотя бы какую-то формулировку правильного поведения, с помощью чисто механических действий (за исключением, возможно, последнего шага) ее можно привести к наиболее лаконичному и наглядному виду.
P.S. Все куски знаний, которые сложились в описанный в публикации алгоритм, были получены автором еще в школе более 15 лет назад. За что он выражает огромную благодарность учителям-энтузиастам, дающим детям основу нормального образования. Татьяна Алексеевна, Наталья Павловна, если вы вдруг это читаете, большое вам СПАСИБО!
