Проблема стиля кодирования: должны ли мы иметь функции, которые принимают параметр, модифицируют его, а затем возвращают этот параметр?

19

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

У нас есть функция, которая принимает параметр, заполняет его член и затем возвращает его:

Item predictPrice(Item item)

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

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

  • Если у вас есть несколько ссылок на элемент, передаваемый (или указатели, или что-то еще), он выделяет новый объект и возвращает его, что имеет материальное значение, так как эти ссылки будут неправильными.

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

  • Выделение в куче потенциально дорого, и поэтому важно, чтобы вызываемая функция это делала.

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

void predictPrice(Item item)

В каждой кодовой базе (по общему признанию кодовой базы C и C ++, а не Java, на которой мы работаем), вышеуказанный стиль по существу придерживался и придерживался значительно более опытными программистами. Он утверждает, что, поскольку мой размер выборки кодовых баз и коллег небольшой из всех возможных кодовых баз и коллег, и, таким образом, мой опыт не является истинным показателем того, является ли кто-то лучшим.

Итак, есть мысли?

Squimmy
источник
strcat изменяет параметр на месте и по-прежнему возвращает его. Должен ли strcat вернуть void?
Джерри Иеремия,
Java и C ++ имеют очень разное поведение в отношении передачи параметров. Если Itemесть class Item ...и нет typedef ...& Item, то местный itemуже копия
Caleth
google.github.io/styleguide/cppguide.html#Output_Parameters Google предпочитает использовать для
возврата

Ответы:

26

Это действительно вопрос мнений, но я не думаю, что возвращать измененный элемент, если он был изменен на месте, вводит в заблуждение. Отдельно, если predictPriceон собирается изменить элемент, у него должно быть имя, указывающее, что он собирается это сделать (например, setPredictPriceили что-то подобное).

Я бы предпочел (по порядку)

  1. Это predictPriceбыл методItem (по разумной причине, чтобы этого не было, что вполне может быть в вашем общем дизайне), который либо

    • Возвращаем прогнозируемую цену или

    • Было имя как setPredictedPriceвместо

  2. Это predictPrice не изменило Item, но вернул предсказанную цену

  3. Это predictPriceбыл voidметод, называемыйsetPredictedPrice

  4. Это predictPriceвернуло this(для цепочки методов к другим методам в любом экземпляре, частью которого он является) (и был вызван setPredictedPrice)

TJ Crowder
источник
1
Спасибо за ответ. Что касается 1, мы не можем иметь предикат цены внутри элемента. 2 - хорошее предложение - я думаю, что это, вероятно, правильное решение здесь.
Squimmy
2
@Squimmy: И конечно же, я провел 15 минут , касающихся ошибки , вызванной кто использует именно шаблон , который вы споря против: a = doSomethingWith(b) модифицирована b и вернул его с изменениями, которые взорвали мой код позже , что думал , что знал , что bбыло в этом. :-)
TJ Crowder
11

В рамках парадигмы объектно-ориентированного проектирования вещи не должны модифицировать объекты вне самого объекта. Любые изменения в состоянии объекта должны быть сделаны с помощью методов объекта.

Таким образом, void predictPrice(Item item)функция члена какого-то другого класса неверна . Возможно, это было бы приемлемо во времена C, но для Java и C ++ модификации объекта подразумевают более глубокую связь с объектом, что, вероятно, приведет к другим проблемам проектирования в будущем (когда вы реорганизуете класс и изменяете его поля, теперь, когда для параметра «ForexPrice» необходимо изменить его в каком-либо другом файле.

Возвращая новый объект, не имеет побочных эффектов, передаваемый параметр не изменяется. Вы (метод предсказания) не знаете, где еще этот параметр используется. Является ли Itemключ к хэш где - нибудь? Вы изменили его хеш-код при этом? Кто-то еще держит его, ожидая, что он не изменится?

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


Давайте посмотрим, что произойдет, если возиться с полями чего-либо в хэше. Давайте возьмем некоторый код:

import java.util.*;

public class Main {
    public static void main (String[] args) {
        Set set = new HashSet();
        Data d = new Data(1,"foo");
        set.add(d);
        set.add(new Data(2,"bar"));
        System.out.println(set.contains(d));
        d.field1 = 2;
        System.out.println(set.contains(d));
    }

    public static class Data {
        int field1;
        String field2;

        Data(int f1, String f2) {
            field1 = f1;
            field2 = f2;
        }

        public int hashCode() {
            return field2.hashCode() + field1;
        }

        public boolean equals(Object o) {
            if(!(o instanceof Data)) return false;
            Data od = (Data)o;
            return od.field1 == this.field1 && od.field2.equals(this.field2);

        }
    }
}

ideone

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

Выход этого кода:

true
false

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

Таким образом, возвращаясь к исходному аспекту вопроса, вызываемый метод не «знает», как используется объект, который он получает в качестве параметра. Предоставление «позволяет мутировать объект» как единственный способ сделать это означает, что есть ряд тонких ошибок, которые могут закрасться. Как потеря значений в хэше ... если вы не добавите больше, и хэш не будет перефразирован - поверьте мне Это ужасная ошибка для поиска (я терял значение, пока не добавил еще 20 элементов в hashMap, а затем внезапно он снова появился).

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

Связанный: перезапись и возврат значения аргумента, используемого как условный оператор if, внутри того же оператора if

Сообщество
источник
3
Это не звучит правильно. Скорее всего, predictPriceне следует напрямую возиться с Itemчленами, но что не так с вызовом методов, Itemкоторые делают это? Если это единственный изменяемый объект, возможно, вы можете указать, что это должен быть метод изменяемого объекта, но в других случаях модифицируется несколько объектов (которые определенно не должны принадлежать к одному и тому же классу), особенно в достаточно высокоуровневые методы.
@delnan Я признаю, что это может быть (чрезмерной) реакцией на ошибку, которую мне когда-то приходилось разыскивать после исчезновения и повторного появления значения в хэше - кто-то возился с полями объекта в хэше после его вставки вместо того, чтобы делать копию объекта для его использования. Существуют меры защитного программирования, которые можно предпринять (например, неизменяемые объекты), но такие вещи, как пересчет значений в самом объекте, когда вы не являетесь «владельцем» объекта, или сам объект - это то, что может легко вернуться к укусу ты жесткий.
Вы знаете, есть хороший аргумент для использования большего количества свободных функций вместо функций класса для улучшения инкапсуляции. Естественно, функция, получающая постоянный объект (как неявный this или параметр), не должна изменять его каким-либо наблюдаемым способом (некоторые постоянные объекты могут кэшировать объекты).
Дедупликатор
2

Я всегда следую практике не мутировать чужой объект. То есть, только мутирующие объекты, принадлежащие классу / struct / whathaveyou, внутри которого вы находитесь.

Учитывая следующий класс данных:

class Item {
    public double price = 0;
}

Хорошо:

class Foo {
    private Item bar = new Item();

    public void predictPrice() {
        bar.price = bar.price + 5; // or something
    }
}

// Elsewhere...

Foo foo = Foo();
foo.predictPrice();
System.out.println(foo.bar.price);
// Since I called predictPrice on Foo, which takes and returns nothing, it's
// apparent something might've changed

И это очень ясно

class Foo {
    private Item bar = new Item();
}
class Baz {
    public double predictPrice(Item item) {
        return item.price + 5; // or something
    }
}

// Elsewhere...

Foo foo = Foo();
Baz baz = Baz();
foo.bar.price = baz.predictPrice(foo.bar);
System.out.println(foo.bar.price);
// Since I explicitly set foo.bar.price to the return value of predictPrice, it's
// obvious foo.bar.price might've changed

Но это слишком грязно и загадочно для моих вкусов:

class Foo {
    private Item bar = new Item();
}
class Baz {
    public void predictPrice(Item item) {
        item.price = item.price + 5; // or something
    }
}

// Elsewhere...

Foo foo = Foo();
Baz baz = Baz();
baz.predictPrice(foo.bar);
System.out.println(foo.bar.price);
// In my head, it doesn't appear that to foo, bar, or price changed. It seems to
// me that, if anything, I've placed a predicted price into baz that's based on
// the value of foo.bar.price
Бен Легжеро
источник
Пожалуйста, сопровождайте любые комментарии вниз, чтобы я знал, как написать лучшие ответы в будущем :)
Бен Leggiero
1

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

В Java Itemэто ссылочный тип - это тип указателей на объекты, которые являются экземплярамиItem . В C ++ этот тип записывается как Item *(синтаксис для одной и той же вещи отличается в разных языках). Так что Item predictPrice(Item item)в Java это эквивалентно Item *predictPrice(Item *item)в C ++. В обоих случаях это функция (или метод), которая берет указатель на объект и возвращает указатель на объект.

В C ++ Item(без чего-либо еще) это тип объекта (при условииItem это имя класса). Значение этого типа «является» объектом и Item predictPrice(Item item)объявляет функцию, которая принимает объект и возвращает объект. Поскольку &он не используется, он передается и возвращается по значению. Это означает, что объект копируется, когда передается, и копируется, когда возвращается. В Java нет эквивалента, потому что в Java нет типов объектов.

Так что это? Многое из того, что вы задаете в этом вопросе (например, «я верю, что это работает с тем же объектом, который передается в ...»), зависит от понимания того, что именно передается и возвращается сюда.

user102008
источник
1

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

void predictPrice(Item * const item);

Этот стиль приводит к:

  • Называя его, вы видите:

    predictPrice (& my_item);

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

  • В противном случае предпочтение передается по const ref или value, если вы не хотите, чтобы функция изменяла экземпляр.
user27886
источник
Следует отметить, что, хотя этот синтаксис намного понятнее, он недоступен в Java.
Бен Легжеро
0

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

Обратите внимание, что он имеет смысл только тогда, когда метод может свободно возвращать другой объект. Если ваш Javadoc говорит, @returns the same value of parameter aто это бесполезно. Но если ваш Javadoc говорит @return an instance that holds the same data than parameter a, то возвращение того же или другого экземпляра - это деталь реализации.

Например, в моем текущем проекте (Java EE) у меня есть бизнес-уровень; хранить в БД (и назначать автоматический ID) сотрудник, скажем, сотрудник, у меня есть что-то вроде

 public Employee createEmployee(Employee employeeData) {
   this.entityManager.persist(employeeData);
   return this.employeeData;
 }

Конечно, нет никакой разницы с этой реализацией, поскольку JPA возвращает тот же объект после назначения идентификатора. Теперь, если я перешел на JDBC

 public Employee createEmployee(Employee employeeData) {
   PreparedStatement ps = this.connection.prepareStatement("INSERT INTO EMPLOYEE(name, surname, data) VALUES(?, ?, ?)");
   ... // set parameters
   ps.execute();
   int id = getIdFromGeneratedKeys(ps);
   ??????
 }

Сейчас на ????? У меня есть три варианта.

  1. Определите сеттер для Id, и я верну тот же объект. Некрасиво, потому что setIdбудет доступно везде.

  2. Сделайте некоторую тяжелую работу по отражению и установите id в Employeeобъекте. Грязный, но, по крайней мере, он ограничен createEmployeeзаписью.

  3. Предоставьте конструктор, который копирует оригинал Employeeи принимает также idнабор.

  4. Извлечь объект из БД (может потребоваться, если в БД рассчитываются значения некоторых полей или если вы хотите заполнить объект недвижимости).

Теперь для 1. и 2. вы можете возвращать один и тот же экземпляр, но для 3. и 4. вы будете возвращать новые экземпляры. Если из принципа, который вы установили в своем API, «реальным» значением будет то, которое возвращает метод, у вас больше свободы.

Единственный реальный аргумент, против которого я могу подумать, заключается в том, что при использовании реализаций 1. или 2. люди, использующие ваш API, могут пропустить присвоение результата, и их код сломается, если вы измените 3. или 4. реализации).

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

SJuan76
источник
0

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

  1. Ровно одна сущность считается «владельцем» изменяемого объекта и рассматривает состояние этого объекта как часть своего собственного. Другие объекты могут содержать ссылки, но должны рассматривать ссылку как идентификацию объекта, принадлежащего кому-то другому.

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

Мне не нравится, когда методы видоизменяют базовый объект и возвращают ссылку, поскольку они создают видимость подгонки второго шаблона выше. Есть несколько случаев, когда подход может быть полезен, но код, который будет мутировать объекты, должен выглядеть так, как будто он это сделает; код вроде myThing = myThing.xyz(q);выглядит намного меньше, как будто он мутирует объект, идентифицируемый myThingчем просто myThing.xyz(q);.

Supercat
источник