LINUX.ORG.RU

Создание микроклассов и антипаттерн полтергейст

 ,


0

2

В книге «Clean Code: A Handbook of Agile Software Craftsmanship» от Robert C. Martin есть такой пример плохого, неочевидного кода:

private void printGuessStatistics(char candidate, int count) {
    String number;
    String verb;
    String pluralModifier;
    if (count == 0) {
      number = "no";
      verb = "are";
      pluralModifier = "s";
    } else if (count == 1) {
      number = "1";
      verb = "is";
      pluralModifier = "";
    } else {
      number = Integer.toString(count);
      verb = "are";
      pluralModifier = "s";
    }
    String guessMessage = String.format(
      "There %s %s %s%s", verb, number, candidate, pluralModifier
    );
    print(guessMessage);
  }

Автором предлагается переделать этот код в отдельный класс:

public class GuessStatisticsMessage {
  private String number;
  private String verb;
  private String pluralModifier;
  public String make(char candidate, int count) {
    createPluralDependentMessageParts(count);
    return String.format(
      "There %s %s %s%s", 
       verb, number, candidate, pluralModifier );
  }
  private void createPluralDependentMessageParts(int count) {
    if (count == 0) {
      thereAreNoLetters();
    } else if (count == 1) {
      thereIsOneLetter();
    } else {
      thereAreManyLetters(count);
    }
  }
  private void thereAreManyLetters(int count) {
    number = Integer.toString(count);
    verb = "are";
    pluralModifier = "s";
  }
  private void thereIsOneLetter() {
    number = "1";
    verb = "is";
    pluralModifier = "";
  }
  private void thereAreNoLetters() {
    number = "no";
    verb = "are";
    pluralModifier = "s";
  }
}

Как вы считаете насколько это оправдано на самом деле? Ведь налицо использование антипаттерна полтергейста, когда создается микрокласс с несколькими простейшими функциями.

В микроклассах нет ничего плохого. Даже наоборот.
Как пример можно привести класс «имя пользователя», который большинство представляет как простую строку, а потом бешенно проверяет его на корректность в куче мест вместо того, чтобы просто сделать что-то вроде этого и получить корректность и строгую типизацию:

public final class Username {
    private final String data;

    public Username(String data) {
        if (data == null)
            throw new IllegalArgumentException("data");

        this.data = data.trim().toLowercase();

        if (this.data.isEmpty() || this.data.length() > 32)
            throw new IllegalArgumentException("data");
    }

    @Override
    public String toString() {
        return data;
    }
}

sanwashere ★★ ()

Сами методы всё равно будут заинлайнены за счёт JIT, так что ничего страшного в этом нету.

sanwashere ★★ ()

Как вы считаете насколько это оправдано на самом деле?

Первый вариант, естественно, гораздо лучше. Второй - типичный ынтерпрайз-онанизм.

tailgunner ★★★★★ ()
Ответ на: комментарий от tailgunner

Первый вариант, естественно, гораздо лучше. Второй - типичный ынтерпрайз-онанизм.

Лорчую.

Deleted ()
Ответ на: комментарий от demensdeum

Указанный вами или мой класс разве «hard to understand» или «hard to maintain»? «limited lifecycle» тоже спорная вещь. Не все классы должны жить вечно. А если есть страх большого потребления памяти, то короткоживущие классы не уходят дальше young generation, и, как следствие, достаточно легко освобождаются во время GC.

sanwashere ★★ ()
Ответ на: комментарий от sanwashere

Ну и вообще, «паттерны головного мозга», - такая же плохая болезнь, как и «Энтерпрайз головного мозга».

sanwashere ★★ ()
Ответ на: комментарий от sanwashere

Мне больше интересно почему Uncle Bob считает что переделывать простейшие функции в микро-нано-классы читаемее и правильнее в своей книге Clean Code. Я сам конечно понимаю что надо отталкиваться от контекста и поставленной задачи, но там это выглядит как правило которому надо следовать всегда, я нахожу это странным.

demensdeum ()
Ответ на: комментарий от demensdeum

Ну, приведённая функция уже не простейшая, IMHO. Содержимое блоков if-elseif уже как раз тянет на простейшие функции, что класс приведённый автором и делает, разбивая её на несколько. Но сам класс выглядит странно, соглашусь. Я бы наверное сделал просто несколько статических методов.

sanwashere ★★ ()
Последнее исправление: sanwashere (всего исправлений: 2)

Ведь налицо использование антипаттерна полтергейста, когда создается микрокласс с несколькими простейшими функциями.

Можно под любой вариант кода подогнать паттерн, либо антипаттерн, было бы желание.

Но поставим вопрос иначе. Вот два куска кода. Действительно ли проще читать второй кусок и понимать происходящее? Ведь, именно в этом суть борьбы с антипаттернами.

По-моему, ясный и однозначный код из первого варианта превращен во втором в неочевидное нагромождение методов.

  private void thereAreManyLetters(int count) {
  ...
  }
  private void thereIsOneLetter() {
  ...
  }
  private void thereAreNoLetters() {
  ...
  }

По одним только названиям видно, что это плохой код. А уж по коду внутри — и подавно. Следующий этап этого дизайна будет выглядеть так:

  private void createPluralDependentMessageParts(int count) {
    applyCount(count);
    if (countIsZero()) {
      thereAreNoLetters();
    } else if (countIsOne()) {
      thereIsOneLetter();
    } else {
      thereAreManyLetters(count);
    }
  }
  private void thereAreManyLetters(int count) {
    applyNumber(Integer.toString(count));
    applyVerbAre();
    applyPluralModifier();
  }
  private void thereIsOneLetter() {
    applyNumberOne();
    applyVerbIs();
    applySingularModifier();
  }
  private void thereAreNoLetters() {
    applyNumberNo();
    applyVerbAre();
    applyPluralModifier();
  }
Deleted ()
Последнее исправление: Deleted (всего исправлений: 1)

всё правильно в книжке написано. я бы ещё и интерфейс добавил

interface {
  void thereAreNoLetters();
  void thereIsOneLetter();
  void thereAreManyLetters(count);
}

а те, кто не понимает зачем это нужно - долбоёбы Ліл :-)

anonymous ()

Как вы считаете насколько это оправдано на самом деле?

Зависит от того, где, как и сколько раз будет использоваться. И чего туда еще собираются навернуть. В отрыве от контекста, класс выглядит оверхедом.

Единсвенно, что ИМХО очень важно: не должно быть вызова print() в printGuessStatistics(). А должен быть возврат String. Тогда, вместе с парой тестов, это будет хороший код.

Deleted ()
Ответ на: комментарий от sanwashere

Содержимое блоков if-elseif уже как раз тянет на простейшие функции

Вообще-то оно тянет на один switch.

tailgunner ★★★★★ ()
Ответ на: комментарий от demensdeum

Я сам конечно понимаю что надо отталкиваться от контекста и поставленной задачи, но там это выглядит как правило которому надо следовать всегда, я нахожу это странным.

Тоже только начал её читать, кстати. У Meyers'a чётко было написано в Effective C++, что его советы работают в большинстве случаев, но ни в коем случае их нельзя воспринимать за догму. Я пока у Uncle Bob не увидел подобной оговорки.

UVV ★★★★★ ()
Ответ на: комментарий от tailgunner

Щас ещё придут немногочисленные адепты секты «switch - это плохо, почти как goto»

UVV ★★★★★ ()

1) пример выглядит очень искусственным, потому что непонятно, почему бы вообще не сделать так (или с case по вкусу):

  private void printGuessStatistics(char candidate, int count) {

    String guessMessage;

    if (count == 0) {
      guessMessage = String.format("There are no %ss",candidate);
    } else if (count == 1) {
      guessMessage = String.format("There is one %s",candidate);
    } else {
      guessMessage = String.format("There is %s %ss",count, candidate);
    }

    println(guessMessage);
  }

2) мне казалось что в таких случаях у труЪ-хардкорных ООП-мужиков в ходу вынос статистики в отдельный интерфейс, который хранит candidate и count уже и формирует такие красивые строчки каким-нить методом.

3) зачем тут вообще класс?

RedPossum ★★★★★ ()

Как вы считаете насколько это оправдано на самом деле? Ведь налицо использование антипаттерна полтергейста, когда создается микрокласс с несколькими простейшими функциями.

В целом не оправданное усложнение мне кажется, там нет только полтергейст - весь букет.

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

anonymous ()

Мне кажется Uncle Bob старый smalltalk'щик там ведь все отдельный класс, отсюда и желание описать этот подход как идеальный.

demensdeum ()
Ответ на: комментарий от tailgunner

В данном случае - да, но в общем - явно нет, ибо условия не всегда будут в виде сравнения с дискретными значениями. Всё-таки это просто пример, хоть и может не совсем удачный.

sanwashere ★★ ()
Последнее исправление: sanwashere (всего исправлений: 1)

Пишите так, как удобнее.

Мне лично, например, «классы без состояния» неудобны.

Второй тип - это даже не Poltergeist, а его подтип т.н.«Gypsy Wagon», а я его условно называю «тупак в стиле Luxoft».

Agile - это глупость несусветная. Заказчик просто сядет вам на голову и будет каждые два дня менять техзадание.

Это, действительно, «псевдо-enterpriZe», абсолютно неоправданный для реального проекта -

https://vc.ru/20942-agile-victims

https://dev.by/lenta/main/pochemu-inogda-agile-vedet-k-provalu

Bioreactor ★★★★★ ()

Clean code — очень ОЧЕНЬ вредная книга для жуниора. Выкинь ее. Если ты задаешь такие вопросы, тебе рано ее читать. В ней автор высказывает свое (по моему мнению недалекое и в 90% ошибочное) мнение. Любой нормальный программист поспорит с КАЖДЫМ тезисом этой книги и выиграет спор.

Выкинь эту книгу и никогда не заглядывай в нее за советом. Ее нахайпили пхп-макаки без мозгов, без образования и без собственного мнения по любому вопросу.

Deleted ()
Последнее исправление: rj45 (всего исправлений: 1)
Ответ на: комментарий от UVV

Щас ещё придут немногочисленные адепты секты «switch - это плохо, почти как goto»

Хм. Никогда таких не видел. Обычно адепты как раз за switch во все поля, где больше двух ифов.

Deleted ()
Последнее исправление: rj45 (всего исправлений: 1)
private abstract class pluralModifier
{
  public abstract string getModifier();
}

private sealed class pluralModifierS
{
  public string pluralModifierS()
  {
    return "s";
  }
}

private sealed class pluralModifierEmpty
{
  public string pluralModifierEmpty()
  {
    return string.Empty;
  }
}

private sealed class pluralModifierFactory
{
  public static Dictionary<Function<int, bool>, pluralModifier> modifiers = new {
    {
      count => count == 1,
      new pluralModifierEmpty();
    },
    {
      count => count == 0 || count > 1,
      new pluralModifierS();
    }
  };

  public pluralModifierFactory(count)
  {
    if (count < 0)
    {
      throw new InvalidArgumentException("Count must be zero or greater");
    }

    Count = count;
  }

  public Count { get; private set; }

  public getPluralModifier()
  {
    foreach (var kvp in modifiers)
    {
      if (kvp.Key(Count))
      {
        return kvp.Value;
      }
    }
  }
}

Ну вот как-то так... и дальше по смыслу ещё раза три по столько. Тогда это будет более-менее настоящее ООП.

anonymous ()
Ответ на: комментарий от anonymous

Ой, я там отнаследоваться забыл. Прошу прощения, пишу с тапка. Там должно быть по смыслу понятно.

anonymous ()
Ответ на: комментарий от RedPossum

+1

Только тут ещё имеет смысл заюзать какой-нибудь аналог gettext, в котором вопросы локализации на основе чисел уже решены.

i-rinat ★★★★★ ()

Я вам покушать принёс.

using System;
using System.Collections.Generic;
using System.Linq;

namespace Rextester
{
    public abstract class Number
    {
          public abstract string getNumber();
    }

    public sealed class ZeroNumber: Number
    {
        public ZeroNumber(int count) {
            if (count != 0) {
                throw new ArgumentException(@"Count must be zero");
            }
        }
        
        public override string getNumber() {
            return @"no";
        }
    }

    public sealed class OneNumber: Number
    {
        public OneNumber(int count) {
            if (count != 1) {
                throw new ArgumentException(@"Count must be 1");
            }
        }
        
        public override string getNumber() {
            return @"1";
        }
    }
    
    public sealed class ManyNumbers: Number
    {
        private int count;
        
        public ManyNumbers(int count) {
               this.count = count;
        }
        
        public override string getNumber() {
            return @"count";
        }
    }
    
    public sealed class NumberFactory
    {
        private static Dictionary<Func<int, bool>, Type> numbers = new Dictionary<Func<int, bool>, Type> {
            {
                count => count == 0,
                typeof(ZeroNumber)
            },
            {
                count => count == 1,
                typeof(OneNumber)
            },
            {
                count => count > 1,
                typeof(ManyNumbers)
            }
        };

        public NumberFactory(int count) {
            if (count < 0) {
                throw new ArgumentException(@"Count must be zero or greater");
            }

            Count = count;
        }

        public int Count { get; private set; }

        public Number getNumber() {
            foreach (var kvp in numbers) {
                if (kvp.Key(Count)) {
					return (Number)Activator.CreateInstance(kvp.Value, Count);
                }
            }
            
            throw new ApplicationException(@"Required number not found");
        }
    }

    public abstract class Verb
    {
        public abstract string getVerb();
    }

    public sealed class ZeroVerb: Verb
    {
        public override string getVerb() {
            return @"are";
        }
    }

    public sealed class OneVerb: Verb
    {
        public override string getVerb() {
            return @"is";
        }
    }
    
    public sealed class ManyVerb: Verb
    {
        public override string getVerb() {
            return @"are";
        }
    }
    
    public sealed class VerbFactory
    {
        private static Dictionary<Func<int, bool>, Verb> verbs = new Dictionary<Func<int, bool>, Verb> {
            {
                count => count == 0,
                new ZeroVerb()
            },
            {
                count => count == 1,
                new OneVerb()
            },
            {
                count => count > 1,
                new ManyVerb()
            }
        };

        public VerbFactory(int count) {
            if (count < 0) {
                throw new ArgumentException(@"Count must be zero or greater");
            }

            Count = count;
        }

        public int Count { get; private set; }

        public Verb getVerb() {
            foreach (var kvp in verbs) {
                if (kvp.Key(Count)) {
                    return kvp.Value;
                }
            }
            
            throw new ApplicationException(@"Required verbs not found");
        }
    }

    public abstract class PluralModifier
    {
          public abstract string getModifier();
    }

    public sealed class PluralModifierS: PluralModifier
    {
        public override string getModifier() {
            return @"s";
        }
    }

    public sealed class PluralModifierEmpty: PluralModifier
    {
        public override string getModifier() {
            return string.Empty;
        }
    }

    public sealed class PluralModifierFactory
    {
        private static Dictionary<Func<int, bool>, PluralModifier> modifiers = new Dictionary<Func<int, bool>, PluralModifier> {
            {
                count => count == 1,
                new PluralModifierEmpty()
            },
            {
                count => count == 0 || count > 1,
                new PluralModifierS()
            }
        };

        public PluralModifierFactory(int count) {
            if (count < 0) {
                throw new ArgumentException(@"Count must be zero or greater");
            }

            Count = count;
        }

        public int Count { get; private set; }

        public PluralModifier getPluralModifier() {
            foreach (var kvp in modifiers) {
                if (kvp.Key(Count)) {
                    return kvp.Value;
                }
            }
            
            throw new ApplicationException(@"Required plural modifier not found");
        }
    }
    
    public sealed class MessageBuilder
    {
        private string candidate;
        private int count;
        
        public MessageBuilder(string candidate, int count) {
            this.candidate = candidate;
            this.count = count;
        }
        
        private string BuildVerb() {
            return new VerbFactory(count).getVerb().getVerb();
        }
        
        private string BuildNumber() {
            return new NumberFactory(count).getNumber().getNumber();
        }
        
        private string BuildPluralModifier() {
            return new PluralModifierFactory(count).getPluralModifier().getModifier();
        }
        
        public string Build() {
            return string.Format(@"There {0} {1} {2}{3}", BuildVerb(), BuildNumber(), candidate, BuildPluralModifier());
        }
    }
    
    public class Program {
        public static void Main(String[] args) {
            Console.WriteLine("Input candidate: ");
            string candidate = Console.ReadLine();
            
            Console.WriteLine("Input count: ");
            int count = int.Parse(Console.ReadLine());
            
            Console.WriteLine(new MessageBuilder(candidate, count).Build());
        }
    }
}
Advanced ()
Ответ на: комментарий от RedPossum

1) пример выглядит очень искусственным, потому что непонятно, почему бы вообще не сделать так (или с case по вкусу):

Там наверное в книге за пару страниц до этого вынесли String.format("There в отдельную строчку, отрефакторили типа. И пошло-поехало.

Deleted ()
Последнее исправление: Deleted (всего исправлений: 2)

Второй вариант проще покрывать тестами. И обломы тестирования будут нагляднее. Он лучше с точки зрения получения результата путем эксплуатирвания каманды метафизических индусов. А относительная простота первого построена на том что он не будет увеличиватся. Я бы не стал на это закладыватся в командной работе.

antares0 ★★★ ()
Ответ на: комментарий от antares0

А относительная простота первого построена на том что он не будет увеличиватся. Я бы не стал на это закладыватся в командной работе.

А что тыдумаешь об этом: Создание микроклассов и антипаттерн полтергейст (комментарий) ? То, что нужно?

tailgunner ★★★★★ ()
Ответ на: комментарий от tailgunner

Его внезапно легко рефакторить в отличае от if..if.. логики на две и более страницы.

То, что нужно?

Нужно декларативное описание правил в всех случаях. Но это сделает всю болтологию ненужной:(

antares0 ★★★ ()
Ответ на: комментарий от antares0

А относительная простота первого построена на том что он не будет увеличиватся. Я бы не стал на это закладыватся в командной работе.

Два брата-акробата: преждевременная оптимизации и преждевременный дизайн.

Deleted ()
Ответ на: комментарий от antares0

Его внезапно легко рефакторить в отличае от if..if.. логики на две и более страницы.

Угу, особенно легко, когда структурно алгоритм меняется, и вместо того, чтобы исправить пару if-ов и добавить/удалить пару переменных, нужно перелопатить пятнадцать функций, 6 из них удалить, 4 объединить, и 5 оставшихся дописать и затем разбить на еще более мелкие с переписыванием логики, имён и списка аргументов.

То, что надо в ынтырпрайзе, и есть за что отчитаться перед работодателем.

Deleted ()
Последнее исправление: Deleted (всего исправлений: 2)
Ответ на: комментарий от Advanced

Не то. Показывай код кодогенератора, которым это сделал. Он ведь максимально обобщён я надеюсь?

GPFault ★★ ()
Ответ на: комментарий от Deleted

Два брата-акробата:

Исключительно закон Мерфи

преждевременный дизайн.

У ТС дизайн внезапно одинаковый и делает одно и тоже

antares0 ★★★ ()
Ответ на: комментарий от RedPossum

Идеально. Повторюсь, глаз мозолит println() - с ним хрен протестишь. Поэтому с патчем на возвращаемое значение String и на опечатку дла случая plural - лучший вариант.

Deleted ()
Ответ на: комментарий от Deleted

исправить пару if-ов и добавить/удалить пару переменных

И весь энтерпрайз с грохотом ..м
Тут все же просто. Если ты пишешь ПО для себя, то и пишеь так что бы успевать от граблей уворачиватся. Если индусы для энтерпрайза то все будет написано так что поправить безболезнео пару if-ов будет фантастикой.
Третий вариант когда грамотные разрабочики готовы отвечать за простои сломаного встечается крайне редко.

antares0 ★★★ ()
Ответ на: комментарий от antares0

Его внезапно легко рефакторить

*facepalm.bat*

Удивительный ЛОР, тут можно встретить мнения, которые в страшных кошмарах не привидятся

Deleted ()
Ответ на: комментарий от antares0

Если индусы для энтерпрайза то все будет написано так что поправить безболезнео пару if-ов будет фантастикой.

А код тех же индусов, но умело обученных оверинжинирингу, станет проще править, ну-ну.

Deleted ()
Последнее исправление: Deleted (всего исправлений: 1)
Ответ на: комментарий от Deleted

Покрывать тестами будет проще. Как использовать тестирование в командной разработке в курсе?

antares0 ★★★ ()
Ответ на: комментарий от antares0

Покрывать тестами будет проще. Как использовать тестирование в командной разработке в курсе?

Что вы собираетесь покрывать тестами — методы типа public string pluralModifierS() { return "s"; }?

Выше уже правильно написано: из printGuessStatistics() нужно вытащить formatGuessStatistics() на 6 строк осмысленного кода — вот её тестами и покроете.

Другое дело, что «мне платят за строчки кода, в том числе и за объем покрытия тестами», тогда оно конечно.

Deleted ()
Ответ на: комментарий от Deleted

повторюсь, глаз мозолит println() - с ним хрен протестишь.

не могу не согласиться

RedPossum ★★★★★ ()
Ответ на: комментарий от sanwashere

В микроклассах нет ничего плохого. Даже наоборот.

если используется часто, то да.

barberry ★★ ()

ИМХО, обратная сторона патернов, они подавляют изобретательность.

barberry ★★ ()
Ответ на: комментарий от GPFault

Не то. Показывай код кодогенератора, которым это сделал. Он ведь максимально обобщён я надеюсь?

Во мне достаточно дури, чтобы сделать это вручную. Но с кодогенератором - это отличная идея! :)

Advanced ()
Ответ на: комментарий от barberry

ИМХО, обратная сторона патернов, они подавляют изобретательность.

Многие области IT давно превратились в ремесло и даже рутину. Тут не только петтерны нужны, тут нужно сажать обезьянок писать код, который будет писать код, как минимум.

Advanced ()

Как вы считаете насколько это оправдано на самом деле?

На самом деле автор мудак. Тут нужен 1 класс с интерфейсом

getNumber()
getVerb()
getPluralModifier()
и контейнер объектов который мапит count на нужный объект. А автор просто завернул исходный говнокод в класс, т.е. по факту ничего не поменялось, if по количеству никуда не делся, варианты также захардкодены.

no-such-file ★★★★★ ()

Искусственный пример. Книгу по паттернам нужно использовать для вдохновения, а не как книгу рецептов.

anonymous ()
Ответ на: комментарий от tailgunner

Просто написано коряво, сам подход-то во многих случаях верен

Deleted ()
Ответ на: комментарий от barberry

Не подавляют. Вы видели хоть одного человека, мыслящего исключительно паттернами? Ну и да - изобретательность иногда перерастает в велосипедостроение

Deleted ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.