Является ли изменение входящего параметра антипаттерном? [закрыто]

60

Я программирую на Java, и я всегда делаю конвертеры вроде этого:

public OtherObject MyObject2OtherObject(MyObject mo){
    ... Do the conversion
    return otherObject;
}

На новом рабочем месте шаблон:

public void MyObject2OtherObject(MyObject mo, OtherObject oo){
    ... Do the conversion
}

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

CsBalazsHungary
источник
4
Правильный ответ для этого будет зависеть от языка, поскольку те, где параметры передачи по значению эффективно превращают их в локальные переменные.
Blrfl
1
Второй шаблон иногда используется в качестве меры эффективности для повторного использования объектов. Предполагая, что ooэто объект, который передается методу, а не указатель на новый объект. Это тот случай, здесь? Если это Java, то, вероятно, это так, если это C ++, это может быть не так
Ричард Тингл
3
Это выглядит как преждевременная оптимизация, да. Я рекомендую вам использовать первую форму в целом, но если вы столкнулись с узким местом в производительности, вы можете использовать вторую форму с комментарием, чтобы оправдать ее . Комментарий не позволит вам и другим взглянуть на этот код, и этот вопрос снова появится снова.
Кин
2
Второй шаблон полезен, когда функция возвращает значение, например, success / fail. Но в этом нет ничего действительно «неправильного». Это действительно зависит от того, где вы хотите создать объект.
GrandmasterB
12
Я бы не назвал функции, реализующие эти два разных шаблона, одинаково.
Кейси

Ответы:

70

Это не антипаттерн, это плохая практика.

Разница между антипаттерном и простой плохой практикой здесь: определение анти-паттерна .

В соответствии с чистым кодексом дяди Боба, новый стиль рабочего места, который вы демонстрируете, является плохой практикой , рудиментарным или до ООП времени.

Аргументы наиболее естественно интерпретируются как входные данные для функции.

Все, что заставляет вас проверять сигнатуру функции, эквивалентно двойному дублю. Это когнитивный разрыв и его следует избегать. Во времена, предшествовавшие объектно-ориентированному программированию, иногда приходилось иметь выходные аргументы. Однако большая часть потребности в выходных аргументах исчезает в ОО-языках

Тулаинс Кордова
источник
7
Я не понимаю, почему определение, с которым вы связались, исключает возможность его признания анти-паттерном.
Сэмюэль Эдвин Уорд
7
Я полагаю, это потому, что обоснование, что «мы сохраняем CPU / mem, не создавая новый объект, вместо этого мы должны переработать тот, который у нас есть, и передать его как arg», очевидно, неверно для всех с серьезным фоном ООП, что это может никогда не образуют шаблон - так что мы никак не можем считать его анти-шаблоном по определению ...
vaxquis
1
Как насчет случая, когда входной параметр представляет собой большую коллекцию объектов, которые необходимо изменить?
Стив Чемберс
Хотя я также предпочитаю вариант 1, что если OtherObjectэто интерфейс? Только вызывающий абонент (надеюсь) знает, какого именно типа он хочет.
user949300
@ SteveChambers Я думаю, что в этом случае дизайн класса или метода плохой, и его следует изменить, чтобы избежать.
piotr.wittchen
16

Цитирую знаменитую книгу Роберта К. Мартина «Чистый код»:

Выходных аргументов следует избегать

Функции должны иметь небольшое количество аргументов

Второй шаблон нарушает оба правила, в частности правило «выходной аргумент». В этом смысле он хуже первого паттерна.

claasz
источник
1
Я бы сказал, что это нарушает первый, многие аргументы означают грязный код, но я думаю, что два аргумента совершенно нормальные. Я думаю, что правило для чистого кода означает, что если вам нужно 5 или 6 входящих данных, вы действительно хотите сделать слишком много в одном методе, поэтому вам следует провести рефакторинг для увеличения читабельности кода и объема ООП.
CsBalazsHungary
2
во всяком случае, я подозреваю, что это не строгий закон, а убедительное предложение. Я знаю, что он не будет работать с примитивными типами, если это распространенный шаблон в проекте, младший получит идеи передать int и ожидать, что он будет изменен как Objects.
CsBalazsHungary
27
Независимо от того, насколько правильным может быть чистый код, я не думаю, что это ценный ответ без объяснения причин , по которым этого следует избегать. Это не заповеди на каменной доске, понимание важно. Этот ответ можно улучшить, предоставив краткое изложение рассуждений в книге и ссылку на главу
Daenyth
3
Ну, черт возьми, если какой-то парень написал это в книге, это должен быть хороший совет для любой мыслимой ситуации. Не нужно думать.
Кейси
2
@emodendroket Но чувак, это парень!
Пьер Арло,
15

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

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

user470365
источник
12
Ну, я могу согласиться, если это узкое место, но где бы я ни работал, узкими местами обычно являются алгоритмы с низкой эффективностью, а не клонирование или создание объектов. Я меньше знаю о разработке игр.
CsBalazsHungary
4
@CsBalazsHungary Я полагаю, что проблемы, связанные с созданием объектов, обычно связаны с выделением памяти и сборкой мусора, что, вероятно, будет следующим уровнем узких мест из-за алгоритмической сложности (особенно в среде с ограниченной памятью, например, в смартфонах и т. П.) ,
JAB
JMonkey также часто видел это, так как это часто критично к производительности. Я никогда не слышал, чтобы это называлось «JavaMonkey»
Ричард Тингл
3
@JAB: GC JVM настроен специально для случая эфемерных объектов. Большие количества очень недолговечных объектов тривиально собрать, и во многих случаях все ваше эфемерное поколение может быть собрано одним движением указателя.
Фоши
2
на самом деле, если нужно создать объект , он не будет эффективным с точки зрения производительности; если блок кода должен быть сильно оптимизирован для скорости, вы должны использовать вместо этого примитивы и собственные массивы; из-за этого я считаю, что утверждение в этом ответе не выполняется.
vaxquis
10

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

Euphoric
источник
Я знаю, что они не эквивалентны. Вы правы, мы сохраняем эти вещи, так что это будет иметь значение. То есть вы говорите, что ни один из них не является антипаттерном, это просто вопрос варианта использования?
CsBalazsHungary
@CsBalazsHungary Я бы сказал, да. Судя по небольшому куску кода, который вы предоставили.
Эйфорическая
Я предполагаю, что это становится более серьезной проблемой, если у вас есть входящий параметр примитива, который, конечно, не будет обновляться, так как, как писал @claasz: его следует избегать, если только он не необходим.
CsBalazsHungary
1
@CsBalazsHungary В случае примитивного аргумента функция даже не будет работать. Так что у вас проблема хуже, чем смена аргументов.
Эйфорическая
Я бы сказал, что младший попадет в ловушку попытки сделать примитивный тип выходным параметром. Так что я бы сказал, что первый конвертер должен быть предпочтительным, если это возможно.
CsBalazsHungary
7

Это действительно зависит от языка.

В Java вторая форма может быть антишаблоном, но некоторые языки по-разному обрабатывают передачу параметров. В Ada и VHDL, например, вместо того , чтобы проход по значению или по ссылке, параметры могут иметь режимы in, out, или in out.

Изменение inили чтение outпараметра является ошибкой , но любые изменения in outпараметра передаются обратно вызывающей стороне.

Таким образом, две формы в Аде (это также законные VHDL)

function MyObject2OtherObject(mo : in MyObject) return OtherObject is
begin
    ... Do the conversion
    return otherObject;
end MyObject2OtherObject;

а также

procedure MyObject2OtherObject(mo : in MyObject; oo : out OtherObject) is
begin
    ... Do the conversion
    oo := ... the conversion result;
end MyObject2OtherObject;

Оба имеют свое применение; процедура может возвращать несколько значений в нескольких Outпараметрах, а функция может возвращать только один результат. Поскольку назначение второго параметра четко указано в объявлении процедуры, не может быть никаких возражений против этой формы. Я склонен предпочесть функцию для удобочитаемости. но будут случаи, когда процедура лучше, например, когда вызывающий объект уже создал объект.

Брайан Драммонд
источник
3

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

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

public void ConvertFoo(Foo from, Foo to) {
    if (can't convert) {
        return;
    }
    ...
}

Foo a;
Foo b = DefaultFoo();
ConvertFoo(a, b);
// If conversion fails, b is unchanged

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

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

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

Причина последовательности довольно слабая, но, к сожалению, она может быть самой распространенной.

  • Возможно, стилевые копы на вашем рабочем месте (или в вашей кодовой базе) происходят из не Java-среды и неохотно меняются.
  • Ваш код мог быть портом с языка, где этот стиль более идиоматичен.
  • Вашей организации может потребоваться поддерживать согласованность API на разных языках, и это был стиль с наименьшим общим знаменателем (это глупо, но это происходит даже для Google ).
  • Или, возможно, стиль имел больше смысла в далеком прошлом и трансформировался в его текущую форму (например, это мог быть шаблон TryParse, но какой-то благими намерениями предшественник удалил возвращаемое значение после того, как обнаружил, что никто его вообще не проверял).
congusbongus
источник
2

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

Недостатками этого способа являются:

  1. Не может использоваться в качестве заводского метода, вызывающая сторона должна знать точный подтип, который OtherObjectему нужен, и составить его заранее.
  2. Не может использоваться с объектами, которые требуют параметров в своем конструкторе, если эти параметры получены MyObject. OtherObjectдолжен быть конструируемым, не зная о MyObject.

Ответ основан на моем опыте в C #, я надеюсь, что логика переводит на Java.

Rotem
источник
Я думаю, с ответственностью, которую вы упомянули, это также создает «труднее увидеть» жизненный цикл объектов. В сложной системе методов я бы предпочел прямую модификацию объекта, а не начинать просматривать весь метод, который мы передаем.
CsBalazsHungary
Это было то, что я думал. Примитивный вид инъекции зависимости
Линдон Уайт
Другое преимущество, если OtherObject является абстрактным или интерфейсом. Функция не знает, какой тип создать, но вызывающая сторона может.
user949300
2

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

Однако, если бы имя метода было Convert(как и должно быть, если мы посмотрим на часть «... сделать преобразование»), я бы сказал, что второй метод не имеет никакого смысла. Вы не конвертируете значение в другое значение, которое уже существует, IMO.

guillaume31
источник