Comments 65
Проблема что некоторые не любят комменты видимо банальна — если это относится к рабочим процессам — то проект сдан, или программист уволился — и это не его проблемы. А если к собственным проектам — то и так всё в голове пока есть интерес.
Думаю в open source сообществах дело с комментами должно обстоять лучше — но сам опыта такого не имею, было бы интересно если бы кто-то написал как там на самаом деле.
Вот интересный мне проект, например: Elixir (ссылка просто на первый файл, там везде так).
Скажете «ну это же целый язык!»? Ну вот ссылка уже на мою библиотеку, тоже случайный файл.
Что должно быть понятно?
class PhoneService
{
//Функция очистки телефона, принимает строку
//Перечислим символы которые нам нужно удалить из строки
//Очистим строку от символов
//Стандартизируем номер телефона, если в начале 8, то заменим ее на 7
//Вернем очищенный телефон
}
===>
public function clear_phone(string $phone) {
$symbols_for_delate = ??;
$cleared_phone = ??;
$standarded_phone = ??;
return $standarded_phone;
}Первый комментарий не нужен — он уже и так написан английским языком. Теперь нам надо вывести следующую переменную на базе предыдущей.
Думаю, это неплохой подход для всяких джунов, у которых ещё не собралось в голове понимание в кучу.
улучшить качество кода за счёт уменьшения его размера в два разаПредлагаете измерять качество кода его размером? Ну так, спорная практика.
необходимость в синхронизации двух одинаковых блоков кода написанных зачем-то на разных языках.Два разных языка — это Вы про собственно код и комментарии к нему? Так комментарии в любом случае должны соответствовать коду, на то их и пишут.
Простите, функция действительно должна только очищать номер телефона?
Delate — доносить
Вместо «standardize», больше подходит «normalise»
«symbols_for_delete» — неграмотно, надо «symbols_for_deletion» или «symbols_to_delete»
Кроме того, зачем-то есть список символов, которые нужно удалять. Что если придёт символ, которого нету в списке?
Почему просто не удалить «все что не цифра» регекспом?
Что если придёт символ, которого нету в списке?
Генерировать ошибку?
Если это был «простой пример» — то надо либо поменять код, либо «идею»
Иначе плохой пример.
Нужно очистить номер телефона от символов вроде '+,-, скобок и пробелов', так, чтобы на выходе получился только набор цифр. Фича нужна для реализации поиска по номеру телефона — введенного любым образом
Вы слышали что-нибудь про TDD?
По сути вы можете сначала написать тесты, в которых будет все возможное поведение вашей функции, затем уже саму имплементацию.
Лично у меня так не получается писать, я несколько раз пробовал так делать, но у меня все равно не получается сначала учесть все ньюансы, а потом написать код. Обычно я пишу функцию, а потом пишу на нее несколько тестов, смотрю на них и на ум приходит еще несколько вариантов, из-за которых я вношу правки в функцию. То есть лично мне проще сначала подумать над алгоритмом как это должно работать в принципе, а потом подумать над ньюансами. Хотя справедливости ради мой код так часто переписывается, что с актуальностью многих тестов постоянно беда.
Тесты тоже нужно документировать. Ну увижу я тест, из которого видно, что перед записью в базу номера из него удаляются пробелы. А зачем непонятно. Может место экономят, например.
Тогда уже BDD
Как уже некоторые отметили, такие комментарии действительно дублируют код. От таких комментариев (да еще и на другом языке) нужно избавляться.
Комментарии нужны, когда самодокументируемый код не справляется с заложенной там семантикой, и/или есть неочевидные способы решения, комментарии в таком случае должны подсказывать почему был выбран именно такой подход.
А от такого комментирования пользы никакой:
// модель пользователя
class UserModel {
// инициализуем
constructor(id, name) {}
// взять данные
getData() {}
// взять кешированные данные
getCachedData() {}
}
Я подобным образом приучаю своих разработчиков писать код, алгоритм также пошагово разбивается — 1 шаг это одна строка в комментариях, а потом каждый шаг превращается в 1 метод с нормальным говорящим названием.
Почему сразу не писать названия методов, вместо комментариев?
Вся проблема в наименовании классов/методов/свойств в том чтобы назвать его так, чтобы без комментариев было понятно что происходит и при этом постараться сохранить терминологию во всем проекте, а не раздувать ее (например в одном месте используем user, в другом client, а подразумевается одно и тоже)
Shakespear же :)
Мне кажется, данные комментарии есть в названиях самих переменных, смысла их писать нет, если только вы не преподаватель по программированию для первых курсов. Есть смысл ставить комментарии только для того чтобы сказать пользователю о том что именно делает ваш метод или чтобы отметить особо сложный код, например сложную реализацию хеширования, желательно со ссылкой на страницу статью о данном алгоритме.
Я бы написал так:
//Приводит номер телефона к нормализованному виду. Пример: 79998889988
//$phone - номер телефона в виде: +7 (999) 888 99 88, 8-999-888-99-88 или 89998889988
public function normalize_phone(string $phone){
$symbols_for_delate = ['+','-','(',')', ' '];
$phone = str_replace($symbols_for_delate, '', $phone);
if( $phone[0] == '8' ){
$phone[0] = '7';
}
return $phone;
}public function normalize_phone(UnnormalizedPhone $phone){
$symbols_for_delate = ['+','-','(',')', ' '];
$normalized = str_replace($symbols_for_delate, '', $phone->value);
if( $normalized[0] == '8' ){
$normalized[0] = '7';
}
return new NormalizedPhone($phone);
}
Этот комментарий просто переедет к типам NormalizedPhone и UnnormalizedPhone, да и мы понимаем, что этот код только лишь для примера, если вы серьезно хотите поработать с номерами телефонов, то там все чуть сложнее.
class NormalizedPhone {
private $phone;
public function __construct($phone) {
$this->phone = $phone;
}
public function __toString() {
$normalized = str_replace(['+','-','(',')', ' '], '', $this->phone);
if( $normalized[0] == '8' )
$normalized[0] = '7';
return $normalized;
}
}
echo new NormalizedPhone("+7(999)123-45-67");
Почему у вас нормализация в __toString, а не в конструкторе? Так у вас при каждом преобразовании в строку будет выполняться нормализация. А если в конструкторе, то один раз — при создании.
Зачем вам в этом конкретном случае хранить сырое представление телефона?
Собстно можно и в конструктор перенести, моя основная мысль была не про это, а про то что если уж TheShock предлагает классы вместо функции, то надо их и использовать как классы, в смысле переделать функцию на классы, а не оставлять все вместе, и функцию и класс
Насколько я понял, TheShock предлагает классы не для замены функции, а для того, чтобы с помощью типов обозначить что принимает и что возвращает эта функция.
Это совсем не значит, что тут сразу же нужно наворачивать конструкторы и переопределять __toString.
если уж TheShock предлагает классы вместо функцииНе обязательно. Я говорю о статической типизации. Она есть что в ФП, что в ООП
Зачем вам в этом конкретном случае хранить сырое представление телефона?Данные всегда лучше хранить сырыми. Если у вас есть проблемы с производительностью именно в этом методе — всегда можно кешировать результат. Единственно, конечно, в том примере toString — неудачное использование метода. На TS можно было бы сделать как-то так, а если, вдруг, будут проблемы с производительностью — раскомментировать мемоайз:
const NotNumberRegexp = /[^\d]/g;
class NormalizedPhone {
constructor (
public readonly value: number
){}
}
class UnnormalizedPhone {
constructor (
public readonly value: string
){}
// @memoize
get normalized () {
let normalized = this.value.replace(NotNumberRegexp, '');
if (normalized[0] == '8') {
normalized = '7' + normalized.substr(1);
}
return new NormalizedPhone(Number(normalized));
}
}
console.log(new UnnormalizedPhone("+7(999)123-45-67").normalized)Так же можно сделать в функциональном стиле, но без брендинга будут проблемы с типизацией (хотя это особенности реализации TS):
const NotNumberRegexp = /[^\d]/g;
type UnnormalizedPhone = string;
type NormalizedPhone = number;
function normalizePhone (phone: UnnormalizedPhone) {
const cleared = phone.replace(NotNumberRegexp, '');
const normalized = cleared[0] == '8'
? ('7' + cleared.substr(1))
: cleared;
return Number(normalized) as NormalizedPhone;
}
var normalizedPhone = normalizePhone("+7(999)123-45-67" as UnnormalizedPhone)
console.log(normalizedPhone);Конечно, возле `
type UnnormalizedPhone` теперь нужно в комментарии описать, что он содержит. Но не в каждой функции, которая ожидает `UnnormalizedPhone` или `NormalizedPhone`//Приводит номер телефона к нормализованному виду. Пример: 79998889988
//$phone - номер телефона в виде: +7 (999) 888 99 88, 8-999-888-99-88 или 89998889988
public function normalize_phone(string $phone){
$phone = str_replace(['+','-','(',')', ' '], '', $phone);
if( $phone[0] == '8' )
$phone[0] = '7';
return $phone;
}Псевдокод хорош когда логика заковыристая, и метод строк на 50...100 и больше. Резонное замечание — в таких случаях применяют декомпозицию. Да, естественно. Но с псевдокодом сделать декомпозицию, поправить интерфейсы, дата-контракты и архитектуру классов можно ещё до того, как начали собственно писать код, а не тогда когда написали и решили что «получается как-то сложновато, надо порефакторить».
При использовании псевдокода никто не заставляет писать код построчно под каждым комментарием, равно как и оставлять комментарии к коду, который самоочевиден. Это вообще не серебряная пуля, а инструмент, который надо применять с умом. Как молоток — хорошая штука, но надо уметь использовать, если не хочешь бить по пальцам.
Автор не привёл примера посложнее, хотя не помешало бы, и тему не очень раскрыл. Но это не значит, что псевдокод плохая штука.
А насчёт «хороший код в комментариях не нуждается» — ну, это хороший, а на практике работать приходится с разным, да и свой получается не всегда идеальным, чего уж там.
Всем добра!
А насчёт «хороший код в комментариях не нуждается» — ну, это хороший, а на практике работать приходится с разным, да и свой получается не всегда идеальным, чего уж там.А на практике люди, которые пишут плохой код — комментарии пишут ещё хуже
У автора проблема в том, что он оставил в конечном варианте плохие комментарии. Если бы автор сначала написал комментарии, а потом написал по ним код и удалил комментарии, то возможно никто бы ничего не сказал. А самое главное, что нужного комментария как раз нет, который бы описывал то, что именно делает функция. Из комментария не понятно, что за очистка телефона такая. То есть в конечном варианте автор оставил не нужные комментарии и не оставил нужных.
Абстрагируясь от технических деталей приведенного в статье примера, не совсем понятно почему достаточно известный "Процесс программирования с псевдокодом (ППП)" (Глава 9, "Совершенный код" С. МакКоннелла) заменен на "Парадигму разработки через комментирование. Idea. Comment-Design. Code. (IC-DC)"?
Если вы задаетесь этим вопросом — то вам просто необходимо, в срочном порядке, прочитать "Совершенный Код" Стив Макконнелл. Там данная концепция более чем раскрыта.
Вдвойне странно, что автор статьи упоминает эту книгу. То есть по сути вместо этого можно было сослаться на конкретную главу в книге (тем более там описан не только сам подход, но и указаны его альтернативы). А также детальнее проработать сам пример примения с технической точки зрения (как минимум добавив методам и переменным более выразительные имена) или найти более подходящий пример где комментарии не являются избыточными и несут дополнительный смысл.
Зачем описывать простейший алгоритм комментарием? Код это сделает надежнее и понятнее. А вот написать:
//Обрабатываем только форматы +7(999)123-45-67 и 8(999)123-45-67 на все остальное кидаем исключение, т.к. пользователи вводят чушь которую мы успешно интерпретируем как телефон и потом возникают проблемы. (ссылока не тикет с таким случаем)имеет смысл
В остальном выглядит как вредные советы. Если в теле функции нужны комментарии, то это скорей всего признак того, что код нужно переписать. Во всех остальных редких случаях это действительно сложная и/или специфичная логика, требующая пояснений.
Ну а про вариант с псевдокодом уже выше упоминали.
1. Боязнь получить вопросы на код-ревью (часто — от ревьюверов, которые недостаточно заинтересованы, чтобы чуть подразобраться, прежде чем спрашивать). Т.е. как бы превентивный ответ на эти [бесполезные?] вопросы.
2. Бедное именование переменных/методов. Если с именованием все в порядке, то комментарии чаще всего не нужны.
3. Отсутствие нормального покрытия тестами, т.е. «боязнь регрессии» («кто-нибудь придет потом и натопчет тут, все сломает»).
За скобками также бедная архитектура (простой код не нуждается в комментариях, а сложного кода почти никогда не бывает при хорошей архитектуре), но это из разряда «лучше быть богатым и здоровым чем бедным и больным» или «ломиться в открытую дверь».
- Практика показывает, что комментарии или имена, говорящие зачем мы что-то делаем, нужны или в коде, или в тестах, а лучше и там, и там. Тот кто удаляет "ненужный" код, удалит или поправит и "ненужный" тест
от ревьюверов, которые недостаточно заинтересованы, чтобы чуть подразобраться
Если ревьюеру нужно «подразбираться» — дело швах, есть там комментарии, или их нет.
Как первоначальное описание идея хорошая, но комментарии имеют свойство устаревать, или хуже, когда комментарий поменяли, а логику нет. Конечно это может отлавливатся на ревью, но все ошибаются.
Так же такой подход может спровоцировать длинные методы. Из-за комментариев сложно будет уследить за длинной самого метода и пропорций код/комментарии.
Это действительно полезно на этапе проектирования, но потом должно уходить в документацию (конфлюенс/javadoc)
Увидев тут, здравые замечания — я скорее всего реализую возможность ведения коммент базы- для тех кому они мешают непосредственно в коде.
Над реализацией еще подумаю — но сама идея будет заключаться в удалении комментариев из кода, с последующим хранением их в базе(например в файловой yaml_db), по команде из терминала. Ну и соответственно возвратом их в код, по команде.
Например делаю класс и комментариями пишу методы какие нужны (что должны получить или сделать), к некоторым методам приписываю какие внутри должны вызываться, это помогает например если за день всё не можешь сделать на следующий день будет ясно что нужно реализовать и общий план по коду.
Мне кажется у автора просто пример слишком простой, а вообще такой метод для меня полезен.
Что бы не переписывать с нуля, портянку старого чужого кода, бывает просматриваю по кускам и сам добавляю комментарии, если задача делается не один день то это полезно, пришел на другой день ещё пересмотрел код закомментировал, а потом в итоге можно сказать чужой код становится твоим.
Список кодов по странам
А теперь угадайте, что сделает этот код?
//Перечислим символы которые нам нужно удалить из строки $symbols_for_delete = ['+','-','(',')', ' ']; //Очистим строку от символов $phone = str_replace($symbols_for_delete, '', $phone); //Стандартизируем номер телефона, если в начале 8, то заменим ее на 7 if( $phone[0] == '8' ){ $phone[0] = '7'; }
Поломает все номера из азиатского региона.
Потому что нельзя сперва отбросить часть условного префикса переменной длины, а потом сделать абсолютную замену первого символа.
Нормальная ссылка на список кодов
К сожалению, редактирование закрылось.
Так просто заменять 8 на 7 неправильно. Надо понимать, что за этим стоит.
8-10-7-903-XXX-XX-XX
8 — выход на междугороднюю связь
10 — выход на международную связь
7 — код страны
903 — код города или оператора
XXX-XX-XX — телефонный номер абонента
Для упращения код 810 заменили на +.
Таким образом, номер с +7 это полный международный номер, а номер начинающийся на 8 это номер в пределах страны. Для Греции например код страны 30 и звонить можно как по номеру 81030… так и по +30… И соответственно, если пользователь ПО в Греции, то 8 должно заменятся на +30, а не на +7.
Если код содержит много комментариев, то это может означать, что качеству самого кода не уделено достаточно внимания. Например, неудачно разбит на методы(в поле зрения разработчика на попадает весь квант логики, приходиться "комментировать"), заложена большая цикломатическая сложность(поясняем, что происходит в третьем вложенном if), переиспользуются переменные(поясняем, что это теперь уже не строка из БД, как двумя экранами назад, а реконструированный из нее data object), ...
Но это не самая большая беда комментариев. На взрослых проектах кодовая база велика, разработчиков много, задач в трекере много, нельзя гарантировать, что один и тот же разработчик может закрыть навсегда какой-то функционал. Он меняется совместно с нуждами бизнеса, где-то уже переиспользуются. Что-то делается в режиме — "Вася в отпуске, нужно вчера, глянь". В общем, комментарии менять просто забывают. Т.е. в код добавляется потенциально ещё одно место, где человек может ошибиться. А значит — ошибка лишь вопрос времени. В итоге, на существенной части кодовой базы содержание комментариев не соответствует коду. Либо просто бессмысленно в силу того, что нашлось более удачное название представления бизнес-сущности в коде.
Реально полезный кейс — это пояснение выбора той или иной логики решения, введение в контекст. Например, в силу переезда с одного технического решения на другое — появляется временная двойственность Api. Во всех новых реализациях используется новое Api, но вот тут понадобилось поддержать обратную совместимость там, где новое Api ещё не достаточно протестировано/не реализует функционал/… В Git будет видно, что коммит новый, а Api используется старое. В итоге, без осознания — "почему", рефакторинг с заменой на новое — убьет кусок кода и бог его знает, где и когда это всплывёт.
В основном, это нужно редко, ибо профессиональный разработчик либо знает бизнес-домен, либо все равно в процессе разбора и будет спрашивать старших товарищей.
Такие дела.
Парадигма разработки через комментирование