При работе над поддержкой незнакомой мне кодовой базы я трачу кучу времени на поиск строк при помощи grep. Даже в проектах, полностью написанных мной, мне нужно много искать: имена функций, сообщения об ошибках, имена классов и тому подобное. Если я не могу найти нужное, то я буду как минимум расстроен, а как максимум могу создать опасную ситуацию, если предположу, что какой-то элемент больше не нужен, ведь я не могу найти ссылок на него в кодовой базе. На основании этих ситуаций я выработал правила, которые позволяют повысить грепабельность кода.
Не разделяйте идентификаторы
Оказалось, что разбиение или динамическое создание идентификаторов — это плохая идея.
Допустим, у нас есть две таблицы базы данных shipping_addresses
, billing_addresses
. Может показаться абсолютно нормальным решением создавать имя таблицы динамически по порядковому типу.
const getTableName = (addressType: 'shipping' | 'billing') => {
return `${addressType}_addresses`
}
Хотя кажется, что это красиво и соответствует DRY, для поддержки это не очень удобно: кто-то неизбежно будет искать в кодовой базе имя shipping_addresses
и пропустит это вхождение.
Отрефакторим код для повышения грепабельности:
const getTableName = (addressType: 'shipping' | 'billing') => {
if (addressType === 'shipping') {
return 'shipping_addresses'
}
if (addressType === 'billing') {
return 'billing_addresses'
}
throw new TypeError('addressType must be billing or shipping')
}
То же самое относится к именам столбцов, полям объектов и, упаси боже, именам методов/функций (в Javascript можно создавать имена классов динамически).
Используйте одни и те же имена для элементов во всём стеке
Не переименовывайте поля на границах приложений, чтобы соответствовать схемам наименований. Очевидный пример: мы импортируем идентификаторы в snake_case в стиле Postgres в код на Javascript, а затем преобразуем их в camelCase. Это усложняет поиск — теперь чтобы найти все вхождения, вам нужно использовать grep для двух строк вместо одной!
const getAddress = async (id: string) => {
const address = await getAddressById(id)
return {
streetName: address.street_name,
zipCode: address.zip_code,
}
}
Лучше пойти более сложным путём и возвращать непосредственно объект:
const getAddress = async (id: string) => {
return await getAddressById(id)
}
Плоская структура лучше вложенной
Я взял этот совет из Дзена Пайтона: при работе с пространствами имён уплощение структур папок/объектов обычно лучше, чем вложенность.
Например, если у вас есть два варианта настройки файлов перевода:
{
"auth": {
"login": {
"title": "Login",
"emailLabel": "Email",
"passwordLabel": "Password",
},
"register":
"title": "Register",
"emailLabel": "Email",
"passwordLabel": "Password",
}
}
}
и
{
"auth.login.title": "Login",
"auth.login.emailLabel": "Email",
"auth.login.passwordLabel": "Password",
"auth.register.title": "Login",
"auth.register.emailLabel": "Email",
"auth.register.passwordLabel": "Password",
}
то выберите второй! Тогда вы с лёгкостью можете находить ключи, ссылки на которые предположительно будут выглядеть так: t('auth.login.title')
.
Или рассмотрим структуру компонентов React. Структура компонентов
./components/AttributeFilterCombobox.tsx
./components/AttributeFilterDialog.tsx
./components/AttributeFilterRating.tsx
./components/AttributeFilterSelect.tsx
предпочтительнее, чем
./components/attribute/filter/Combobox.tsx
./components/attribute/filter/Dialog.tsx
./components/attribute/filter/Rating.tsx
./components/attribute/filter/Select.tsx
с точки зрения грепабельности, потому что вы сможете выполнять поиск grep всего компонента AttributeFilterCombobox
в пространстве имён просто по его использованию, а не по строке Dialog
, которая может встречаться в приложении во многих местах.
Комментарии (29)
Aquahawk
03.09.2024 17:24+22Предвижу холивар, но я согласен с автором. Когда мы говорим об одном проекте, да ещё и в нормально типизированном языке, проблем обычно нет. Но когда расследуешь инцидент в котором данные по всему пути проходят через 20-30 реп, когда всего этих реп сотни, грепабильность начинает иметь значение, и не только грепабильность, но и дебагаемость, не счесть сколько раз я в процессе дебага разворачивал однострочные ретурны с тернарным оператором. А ещё я могу написать код вида
const criteria1 = calculateCriteria1(); const criteria2 = calculateCriteria2(); if (criteria1 && criteria2) { return true; } return false;
И когда ты второй час в дебаге в сложно воспроизводимом кейсе ох как помогает возможность брякнуться на нужной ветке ретёрна. И не говорите мне про кондишнал бряки, они не всегда доступны и иногда настолько тормозны что ими нельзя пользоваться.
Возможно это профдеформация с момента как я начал заниматься кросспроектными проблемами, когда кода много, он мало знаком, на разных языках и разных платформах, и вообще иногда чужой и минифицирован.adeshere
03.09.2024 17:24+4А ещё я могу написать код вида (...)
Вы не поверите, но я именно в целях отладки почти дословно делаю то же самое на
слабонервным не смотреть
древнем фортране
В общем, я бы не стал свысока отмахиваться от советов, которые оказываются полезны в настолько ортогональных условиях...
BugM
03.09.2024 17:24Никакого холивара. Он полностью прав. Холивар может быть о следствиях этой статьи. Ну там Джава читаемее и грепабельнее Раста и значит лучше в общем случае. Котлин в общем случае не нужен. И все такое.
CaptainFlint
03.09.2024 17:24+5Тогда вы с лёгкостью можете находить ключи, ссылки на которые предположительно будут выглядеть так:
t('auth.login.title')
Зато итерирование по ключам разных уровней превращается в занимательные приседания.
BugM
03.09.2024 17:24Вы эти ключи все равно читаете в какую-то мапу при старте приложения. Ну и положите в такую структуру чтобы удобно итерироваться было. Да, можно хранить обе структуры при необходимости. Этот расход памяти настолько незначителен что можно не обращать внимания. Или можно перекладывать в нужную структуру если она используется редко. Это в общем случае настолько быстро что не повлияет ни на что.
Код он для чтения человеком, а не чтобы итерироваться удобно было.
CaptainFlint
03.09.2024 17:24+6Ну вот мне как человеку гораздо удобнее видеть иерархическую структуру в коде, а не сплошной плоский дамп всех мыслимых значений. Ну да, грепать сложнее, но, во-первых, настолько ли большая проблема грепать по фразе
['auth']['login']['title']
вместоauth.login.title
? Во-вторых, если это по своей природе иерархическая структура, то наверняка в коде будут не обращения напрямую кauth.login.title
, а предварительное формирование объекта auth и обращение уже к конкретным его дочерним полям. Такое фиг погрепаешь, несмотря на казалось бы грепабельную исходную систему хранения данных.Всё-таки, насколько я понимаю, пример был полностью искусственный и никак не отражал реальное применение. В конце концов, структура определяется не тем, что мы будем грепать, а тем, как нам надо работать с этими данными в коде программы. Если там по принципу работы плоский список независимых переменных, ну так он и будет плоским списком. А если нужна структурированная информация, где работа ведётся с отдельными самостоятельными блоками данных, то правильнее будет её и описывать структурами этих блоков.
VADemon
03.09.2024 17:24на поиск строк при помощи grep.
А с другой стороны, это говорит о том, что нет специализованного инструмента вместо простого поиска по тексту.
Но когда расследуешь инцидент в котором данные по всему пути проходят через 20-30 реп
Правда тут и текущий workflow IDE не поможет, потому что сначала надо подгружать проект, чтобы стал доступен поиск по типам...
С другой стороны существуют поисковики по кодовой базе как https://searchfox.org/, https://livegrep.com/search/linux, https://sourcegraph.com/docs/code-search/queries, но и они не токенизируют код, а просто дают удобный интерфейс с regexp.
И напомню про наличие грепалок в самом git.
vvzvlad
03.09.2024 17:24+9const getTableName = (addressType: 'shipping' | 'billing') => { if (addressType === 'shipping') { return 'shipping_addresses' } if (addressType === 'billing') { return 'billing_addresses' } throw new TypeError('addressType must be billing or shipping') }
Ееее, замечательный способ при добавлении-изменении полей получить баг, отредактировав в одном месте, и не отредактировав по запарке в другом. Теперь, блин, вместо одной правки, слово billing надо править аж в трех местах.
Офигенная оптимизация (нет). КГ/АМ.
Тоже самое про второй пункт. Когда у вас вложенная структура, то вам надо имя родителя менять в одном месте. Когда плоская — в десятке, и легко проглядеть. Особенно, когда они идут не одним блоком, а кто-то там добавил в конце еще один ключ.
"auth.login.title": "Login",
"auth.login.emailLabel": "Email",
"auth.login.passwordLabel": "Password",
"auth.register.title": "Login",
"auth.register.emailLabel": "Email",
"auth.register.passwordLabel": "Password",
"auth.register.name": "Password",
"auth.login.name": "Login",Вот такая структура при переименовании login рискует превратиться в
"auth.auth.title": "Login",
"auth.auth.emailLabel": "Email",
"auth.auth.passwordLabel": "Password",
"auth.register.title": "Login",
"auth.register.emailLabel": "Email",
"auth.register.passwordLabel": "Password",
"auth.register.name": "Password",
"auth.login.name": "Login",
Где у auth никакого name нет, потому что оно осталось у login. Иерархическая схема такого не позволит сделать принципиально. Классическое "удачной отладки, суки".Skubent
03.09.2024 17:24А еще можно не писать billing, оставить только b.
Код - он для чтения, «экономия» времени «на написание» никогда не является экономией.
vvzvlad
03.09.2024 17:24+4А вы комментарий-то читали? Причем тут экономия времени на написание? Речь о том, что сделать ошибку в десяти перечислениях гораздо проще, чем в двух. Ну просто проще, человеки склонны ошибаться. Нет, "ну вы не делайте ошибку" не сработает. Вы не сделаете, ваш коллега сделает. Новый джун сделает, которому доверили простую задачу — поменять название переменной. Вон он обязательно ошибется, и обеспечит уже сеньору пять часов отладки в поисках бага. Вот тут-то ему грепабельность и пригодится..
piton_nsk
03.09.2024 17:24Речь о том, что сделать ошибку в десяти перечислениях гораздо проще, чем в двух.
Поэтому что первый, что второй варианты плохие. Названия таблиц и полей (если уж хочется динамически собирать запросы) должны быть определены как константы в одном единственном месте. Тоже самое и для addressType. Ну и во втором варианте хотя бы ошибка выскакивает, а в первом черт знает что может получится.
dom1n1k
03.09.2024 17:24+6Вот у меня примерно те же мысли.
Трудно спорить, что при прочих равных "грепабельность" лучше её отсутствия. Но жертвуя при этом надёжностью при модификациях – не, спасибо, не хочется.
Взять этот самый пример сshipping | billing
– с двумя опциями это выглядит ок. Но мы же понимаем, что это стерильный пример, а в реальности их окажется 10 или 20 или хз сколько. Аналогично с ключами опций.И да, доведённый до крайности DRY тоже штука сомнительная, но здесь, как мне думается, пока ещё не этот случай.
AzatJ
03.09.2024 17:24+3Проблема этого подхода не столько "грепабельность", сколько внедрение неявной зависимости между БД и кодом. Тип адреса и название таблицы имеют один паттерн по случайности. Завтра мы захотим на фронтенде сделать галочку "Same as shipping address" и у нас проблемка
Dmitri-D
03.09.2024 17:24+3"это все придумал черчилль в восемнадцатом году"
Есть концепция Clean Code - всё в ней, всё в ней. Идеала нет, но это точно большой шаг вперед. А вообще я советую начать с правил форматирования и именования рекомендованные гуглом по всем широко используемым языкам. Эти правила уже нормализуют представление. Clean Code еще и заставляет приводить мысли в порядок.
Применительно к себе могу сказать, что обычно на 3й итерации переписывания код получается достаточно Clean - т.е. другие могут с ходу читать и понимать, включая меня самого через несколько лет.
panzerfaust
03.09.2024 17:24+2const getTableName = (addressType: 'shipping' | 'billing') => { return `${addressType}_addresses` }
Не знаю, выдумал ли это автор, но я такой код прям видел и не раз. И каждый раз за авторством джунов после довольно неплохих вузов. Где-то в недрах высшего ИТ образования сидит поехавший вредитель с идеей "короче - значит круче". Мол если утрамбовал 10 строчек в один лихой однострочник, то сразу }{@ck3R
toxicdream
03.09.2024 17:24Так понимаю, тернарный оператор вам тоже не нравится?
Тогда добро пожаловать в клуб :)
panzerfaust
03.09.2024 17:24Нет, обычный тернарник меня не огорчает. Вложенный - да, уже тяжелее. Самая дичь это что-то типа
fun0(fun1(fun2(fun3(fun4(agrs.reduce {x,y -> reduceFun(x,y)}))))
Кто-то считает, что это прям шик. Я считаю, что это невозможно дебажить.
janatem
03.09.2024 17:24+5Замечание по переводу. Лучше выглядит (да и как-то стандартей, если поискать похожие примеры) "грепабельность" без удвоения согласной. По-английски, ясен пень, удвоение нужно, чтобы сохранить закрытость слога.
TimReset
03.09.2024 17:24+1Ха ха, прям флешбеки - в приложение используется проперти типа map.get("a.b.c.d")
Ну и я пытаюсь найти строку "a.b.c.d". А её нигде нет! Но благо я уже к тому времени был знаком с yaml конфигами так что искал "a" в yaml файлах. А потом уже искал в том файле b и так пока не нашёл нужное. Ппц как неудобно! Для примера property файлы - ищешь "a.b.c.d=" и тебе сразу в окне поиска показываются все дефинишны этой проперти.
OlegZH
А теперь вопрос: что будет. если разработка системы ведётся на одном языке программирования, а реализация — на другом языке программирования? В этом случае. очевидно, вся кодовая база будет под рукой.
Aquahawk
разработка на одном а реализация на другом это как? Много разных языков как раз удачно женятся грепом, а ещё ack и вообще ide с нормальным поиском.
flancer
TypeScript в сорцах => JavaScript в билде
Aquahawk
и как раз значения все будут грепаться отлично, ничего существенно в процессе билда не меняется, только типы пропадают
flancer
И на фронте в бандле тоже ничего существенно не меняется?
domix32
А как и зачем вы фронт грепаете? Натравливаете его на выхлоп curl?
flancer
Как-то так: