Как очистить карму своему коду?

[@LeNsTR]

Ох вас вштырило то как…

[@odiszapc]

Он поймал дзен. И дзен его не отпустил.
А вот тех, кто подтирает комментарии в коде, наказал бы бы.

[@bildja]

Ну, к методу getYaxisMax комментарий «retrieve max value for y-axis» как бы и не очень нужен

[@odiszapc]

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

getYaxisMax — нифига не понятно, что она делает. Что то-там Y, что-то Max. Наверно максимум Y какой-то. Но что за Y, в контексте чего он используется, для чего вообще функция. Не ну пара строк кода внутри функции дают понимание этого, но на них еще нужно посмотреть и понять (а если строк больше?). Так вот, если бы был комментарий из пары строчек, даже самых очевидных, то время затраченное на понимание смысла функции было бы меньше. Из вот таких вот мелочей и складывается удобство.

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

[@asolntsev]

> каждая функция должна быть четко документирована
Я тоже так считаю, но проблема обычной (текстовой) документации в том, что она может врать. Компьютер не умеет проверять её правильность.
Поэтому и предлагаю использовать юнит-тесты — ведь это по сути такая же документация, только её правильность гарантирована тем, что тесты запускаются автоматически при каждой сборке.

[@braindamaged]

Как бэ так:

[@Screatch]

Мне того же отсыпьте что и Вам ;)

[@ad_Wolf]

Немного непонятно сравнение большого количества WTF'ов с PHP и Perl'ом. Имеется ввиду степень свободы этих языков?

[@asolntsev]

В данном контексте имелось в виду, что Scala — хороший язык, Java — так себе, а PHP и Perl — совсем фуфло. Естественно, эта оценка несерьёзная в данном случае. Шутка юмора, как говорится.

[@iscariot]

Поддерживал как-то проект, написанный каким-то «гением» программирования. Надеюсь, что от количества моих WTF во время бессонных ночей, его код никогда не переродится. Пусть лучше йогой занимается.

[@Shchvova]

Блин, а как все начиналось. А скатилось к тестам. Я бы сделал другой вывод. Кстате, а каким образом юнит тест предотвратит WTF? Или он превратит его в «how(why) the fuck it works?!»?

[@Shchvova]

а в целом хорошо и весело написано, крайне согласен с выводами.

[@asolntsev]

> каким образом юнит тест предотвратит WTF?
Ну как каким? Программист заглянет в тест и увидит, как на самом деле работает этот метод. А самое главное, что (в отличие от документации) он может быть уверен, что ето ПРАВДА. В смысле что метод действительно работает именно так, как написано в юнит-тесте.

[@Shchvova]

Юнит тесты никогда ничего не объясняют. Они сопоставляют действия и результаты. Простой пример:
F(5) = 15
F(8) = 18
Это очень хорошо объясняет внетренне устройство F(x), и поможет изменить или починить ее.

[@asolntsev]

Позвольте, на таком уровне «обяснять» должно название метода.
Если у вас метод называется «F()», а о его предназначении можно узнать только из комментариев — извините, это традиционно считается плохом стилем.

[@Shchvova]

замените F на CalculatePopulationOfPlanet(int planetRadius)
стало очевиднее как работает? Нифига не стало. Юнит тесты не несут в себе информации о том "как работает что-то". Они только совопоставляют пары результатов и параметров. Так что «комментирование» кода unit тестами это полный буллшит

[@asolntsev]

Ну как, вообще-то стало. :)

Ну хорошо, а по-вашему, от чего мне станет очевидно, как работает функция CalculatePopulationOfPlanet?

[@Shchvova]

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

[@asolntsev]

Вы не ответили на вопрос.
Oт чего же станет очевидно, как работает функция CalculatePopulationOfPlanet?

[@Shchvova]

ответ: «никак». Ничего кроме комментариев в коде не поможет понять, или, в случае необходимости, поправить. Не помогут тесты, не поможет имя функции.
простой пример когда тесты в качестве документации просто дерьмо, хотя вполне выполняют свою нормальную функцию.

[@asolntsev]

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

[@iviv]

В приведенном в статье коде, ни тесты, ни JavaDoc к методу, не могут и/или не вправе объяснять РЕАЛИЗАЦИЮ метода (а именно на нее ты ругнулся).

Внешним пользователям кода не нужно знать потроха метода. Им достаточно знать что он делает.

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

[@asolntsev]

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

[@iviv]

Не справляются.

1. В JavaDoc не место описанию деталей реализации кода. Надеюсь ты тут согласен.

2. Тесты неочевидны и относятся ко всему методу, а он может быть большим и содержать серию трюков.
Плюс не факт, что код проходит тест — пока не запустишь не узнаешь.

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

Тесты не панацея. Они не предназначены для объяснения деталей реализации методов. И не надо им приписывать несуществующие свойства.

[@asolntsev]

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

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

[@iviv]

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

[@toshnya]

TDD головного мозга. Когда в руках молоток, все вокруг кажется гвоздями.
Вместо того, чтобы написать понятный код:
return TruncateToLower100(GetMinAmount())
нужно обвешивать непонятный код набором неочевидных тестов.

[@asolntsev]

Не всегда так можно сделать.
Возможно, функции getYaxisMin и getYaxisMax должны называться именно так, потому что класс реализует какой-то интерфейс. А округляет он до 100 или, скажем, до 10 — это детали реализации, они могут поменяться со временем. Не переименовывать же методы каждый раз.

[@toshnya]

लेकिन लेखन कोड का एक बहुत कुछ!

[@Xitsa]

Google переводит с хинди как «Но много писать код!» :)

[@dbarashev]

Из реализации интерфейса всё же можно просто вызвать другой метод, более внятно названный. Типа

int getYaxisMin() {
return truncate(min, 100);
}

[@asolntsev]

Конечно проще. Но не лучше.
Сегодня ты напишешь такой комментарий, а завтра кто-то решит, что правильнее будет округлять не до 100, а до 10, или лучше вообще не округлять. А комментарий забудет исправить (я же это не придумал, это случается постоянно). И получится:

[@asolntsev]

//Этот код округляет до сотен в меньшую сторону
return getMaxAmount();

и будут новые WTF'и.

[@iviv]

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

// Поскольку речь идёт о целых числах, то при делении на 100 результат округляется, а при повторном умножении на сто получается меньшее число. Например, из 666 получается (666/100) * 100 = 6*100 = 600. То есть значение округляется до ближайшей сотни.
return getMaxAmount() + 100 ) / 100 * 100;

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

По-хорошему, нужно переписать именно код. Например, заменить цифры на говорящие константы, создать метод округления до заданного порядка и т.п.

[@asolntsev]

Да конечно надо, кто ж спорит!
Но одно другому не мешает. Если вы написали «красивый» код с говорящими константами и т.д., его что же, не надо теперь тестировать?

[@iviv]

Вот тут вы и попались, сэр :)

Никто не отрицает полезность тестов. Но эта статья ведь о целительной силе тестов в деле читабельности кода: я потратил силы, чтобы в нём разобраться и в конце теперь карма вашего кода немного очистилась. И именно это утверждение опровергаю.

[@asolntsev]

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

Попались? :)
Да у меня и не было цели «не попадаться»…

[@iviv]

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

[@asolntsev]

Камон! Это же просто пример.

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

[@iviv]

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

[@verych]

Согласен. Такие вещи надо пояснять (они как бы не для средних умов). И по-хорошему — вынести как метод в хелпер класс, сделав 100 — параметров.

[@Iv8]

В скобочной структуре ошибка. В тексте статьи тоже.

[@professor_k]

Есть такое правило рефакторинга: если хочешь написать комент — выдели функцию. Ну и от магических чисел избавиться. В даном случае что-то вроде:

const int RoundingStep = 100;

public int getYaxisMax() {
return trunkToClosestUpperMultiplierOf(getMaxAmount(), RoundingStep);
}

int trunkToClosestUpperMultiplierOf(int value, int step) {
return (value + step - 1) / step * step;
}


[@asolntsev]

А другое правило рефакторинга гласит: не начинай рефакторинг, пока у тебя нет юнит-тестов для этого кода. :)

[@gorinich]

Главное потом не ошибиться в названии функции %)

Это не рефакторинг, это прям обфускация какая-то…

[@professor_k]

Да ладно… Никто сейчас такие имена полностью не пишет, IDEшки достаточно умны, автодоплнение уже есть всюду — в Visual Studio это називается Intellisence, в Eclipse — не знаю как, но тоже точно есть. О писателях в блокнотах конечно речь не идет. А из плюсов — сразу более-менее понятно что функция делает из самого названия.

[@asolntsev]

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

[@shir]

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

[@asolntsev]

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

[@A1lfeG]

Код недорефакторен.
Похорошему надо было выделить метод TruncateToLower100(), а затем ещё раз писать тесты )
А то эти повторющиеся деления и умножения повышают карму :)

[@A1lfeG]

Ну и как параметр значение 100.

[@propovednik]

return getMaxAmount() + 100 ) / 100 * 100; //скобочку надо

[@Mercury13]

А тут бажок…

result = ( result + 99) / 100 * 100;

[@asolntsev]

Спасибо, что заметили :)

В том-то и дело, что не бажок. Автор именно так и хотел, чтобы метод getYaxisMax(100) возвращал 200. Что и задокументировано в тесте совершенно четко. А в документации — нет.

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

[@professor_k]

Если автору нужен был именно такой подход, то достаточно было сделать вот так:
public int getYaxisMax() {
return getYaxisMin() + 100;
}


ИМХО в таком варианте много понятнее, чего ожидается в граничных случаях.

[@asolntsev]

Вы что-то путаете. В этом случае метод будет работать неправильно.
например, getYaxisMin() может возвращать 200, a getYaxisMax() должен возвращать 1200.

[@professor_k]

Простите, не заметил, что базовые значения для преобразования берутся разные. Имелось в виду, что слагаемое 100 во втором случае можна вынести из числителя, и тогда оба преобразования становятся одинаковыми.