MasterZivso fuck code review!

"Не любите кошечек? Это вы просто не умеете их готовить".
Только что мой код жестоко отревьювили. В результате штук 10 классов поменяли своё имя и в результате всё стало более понятным (в процессе обобщения и рефакторинга некоторые имена да, сильно отстали от смысла).
А если в команде нет согласия, или квалификация ревьюверов низка- то да, фак...

По первоначальному вопросу- да, return "из середины" применим во вполне конкретных местах.

И самое полезное- контроль входных параметров, или состояния. Не то- сразу вернули ответ (exception-то понятно).

Если checkSomeCondition проверяет применимость getValue, то логично после него кидать ответ.

А вообще метод выглядит некрасиво что до, что после рефакторинга.

cdtyjv MasterZiv ,
Нормальный пример. Проблема заключается в том, что чем более "ненадуманным" является пример, тем больше ненужного контекста он вынужден за собой тянуть. Из-за чего воспринимать такой пример становится сложнее.

Хороший метод должен быть не сложнее.

NiticaНадуманный? Хе-хе. Это реальный пример, из реальной системы, метод написан уже несколько лет назад, и в продакшене :)

Код плохой, но не в return дело.
"// do some work here" явно лишнее тут. Не должен геттер после вычисления значения что-то делать.
Если этот кусок кода убрать, то код можно сильно упростить:



Если же почитать весь код и убрать не относящиеся к результату действия, то всё может стать ещё проще.

Вообще же правильно сказано "если в языке все советуют делать ТАК, но можно и ПО ДРУГОМУ значит язык плохо спроектирован".
В eiffel (и по его мотивам в Delphi) есть предопределённая переменная Result, что заставляет, по сути, делать single return :)
В java этого, как и многого другого, нет, поэтому всё решается вкусом техлида.

cdtyjvAlexey TominНе должен геттер после вычисления значения что-то делать.Геттер может делать все, что угодно. Отходите от этих клише, что геттер/сеттер - это обязательно бессмысленные однострочечные. Именно из-за этих шаблонов над Java и смеются - громоздкость на пустом месте.

Геттер не должен менять состояние объекта, и, тем более, других объектов.
Так что если результат известен, то после него должен выполнятся только return.

Иначе- смеяться будет сам разработчик, причём очень нервным смехом. Когда поймает багу.

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

Делая не так, как советуют, надо понимать, зачем.


cdtyjvВот вы говорите, что геттер не может изменять состояния объекта. Действительно, в ряде случаев так и должно быть.
Ну а что, если я хочу, что бы у меня был синглтон с отложеной инициализацией?

Это принятое исключение из правил, связанный с отсутствием в java одноразовых функций (опять же eiffel).

cdtyjvА если я делаю кэширующий прокси? Первым гетом я вызываю какую-нибудь тяжелую операцию, потом кэширую ее результат, и в следующий раз сразу его верну. Тоже изменение состояния.

Не отличается от первого примера :)

cdtyjv А почему я, например, не могу в геттер логировать или учитывать количество его вызовов?

Опять же понятное исключение. Правила они общие. Их надо сначала понять, а потом понять их границы. Если же сразу говорить "откажитесь от шаблонов"- то будет плохо.

NiticaДело не в геттере, я бы мог назвать метод и как-нибудь так: checkMyFavoriteConditions.

Плохое название. Что _возвращает_ метод "check..."?

NiticaИ локальная непроинициализованная переменная в вашем примере свалит компиляцию :)

Я ж изменил комментарий- в первой половине if'а надо её выставить.

Вообще хороший код это не сферический код в вакууме. Изменение имени метода, к примеру, может превратить хорошую функцию в безобразную :)

Например checkMyFavoriteConditions логично быть void и, если что не так, кидаться exception'ом.

Йуный джавистЪТут проблема в убожестве явы как языка программирования. Вот как этот код выглядит на языке программирования coffeescript.




Такие вещи можно сделать много где. А проще всего в eiffel - в заголовке функции пишешь once и она будет вызвана один раз, а потом будет выдаваться закэшированный результат.

PS: в многопоточном коде это точно сработает?

Petro123Alexey TominТакие вещи можно сделать много где. А проще всего в eiffel - в заголовке функции пишешь once и она будет вызвана один раз, а потом будет выдаваться закэшированный результат.
т.е. программисту В КОДЕ нельзя написать один вызов руками и использовать результат во многих местах?
Или я не понял этот сахар?

Не понял.
Обычная функция:



Сколько раз в коде встретится вызов- столько раз и будет вызвана.

Одноразовая на каждый поток:



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

А вот одноразовая на приложение



По сути- встроенная в язык ленивая инициализация.