Pull to refresh

Comments 111

Беру первый же пример
соблюдайте правило вертикали — части одного запроса или условия должны находиться на одном отступе

чуть-чуть изменил ваш код, добавив в if вызов метода
if (typeof a ! == "undefined" &&
    typeof b ! == "undefined" &&
    typeof c === "string") { 
    call_function(a, b, c);
}


Как с читаемостью? Легко видите где кончается условие издалека? :)
Пример приведён, как мне кажется, чисто для демонстрации вертикали, ведь явно обозначено, что после цикла может быть любой код.
А нефиг устраивать отсутпы в 4 символа.
if (typeof a ! == "undefined" &&
    typeof b ! == "undefined" &&
    typeof c === "string") { 
  call_function(a, b, c);
}
Никаких проблем с читабельностью.
А если у него отступы делаются табуляциями? Вообще любой нормальный редактор при длине отступа в 4 символа сделает так:

if (typeof a ! == "undefined" &&
        typeof b ! == "undefined" &&
        typeof c === "string") { 
    call_function(a, b, c);
}

Фактически автоформатирование нарушает это правило.
Вполне удобочитаемо даже с автоформатированием, если перенести скобку в новую строку: сразу видно где условие, а где блок, он отделяется пустой строкой. Я подсознательно сперва пропущу условие, если надо искать только ключевые вызовы функций. А в вашем примере прийдётся сперва выяснить, относится вызов к условию или уже к блоку if.
if (typeof a ! == "undefined" &&
        typeof b ! == "undefined" &&
        typeof c === "string") 
{ 
    call_function(a, b, c);
}

Да бросьте, вам действительно удобно читать такой перенос скобки? Может, конечно, дело привычки, но мой взгляд на ней спотыкается.
А для меня как раз удивительно, когда открывающую скобку оставляют в конце строки. Дело привычки.
У нас в компании код-стандарт как раз такой, как в комментарии у romy4
Конечно, удобно. Сразу видно, где закончилось условие, и где началось тело. Недели за 2 взгляд привыкает, проверено.
Так где условие кончается — и так видно (многоуровневое условие должно иметь другой уровень отступа, нежели код в блоке условия), а вот бесполезно истраченная строка… меня вот смущает.
Так где условие кончается — и так видно

Нет, не видно. Когда условие записано в 4-5 строк, сам if теряется, как и действие.

Вообще, IDE с подсветкой блоков (там где цветными линиями отбиваются блоки) решает все эти проблемы. Очень удобно. Хотя, стайлгайдов не отменяет, конечно.

а вот бесполезно истраченная строка… меня вот смущает.

По-моему, это бессмысленный термин, извините.
По-моему, это бессмысленный термин, извините.
Что значит «бессмысленный»? У вас экран бесконечную высоту имеет или вы всё на слух воспринимаете?
Нет, не видно. Когда условие записано в 4-5 строк, сам if теряется, как и действие.
По-моему вы — уникальный человек: способны запомнить — что там творится в коде, который не вместился на экран, но при этом неспособны понять где в «4-5 строках» начинается и кончается условие.

Хотя есть у меня подозрение: если код не пытаться читать и понимать, а просто пролистывать в надежде «увидеть знакомые буквы», то, возможно, ваши предпочтения становятся осмысленными, но тогда вы навечно обречены порождать нечто, что работает криво и валится по поводу и без повода (тесты — не замена пониманию ни разу!). Вы действительно этого хотите?
Что значит «бессмысленный»? У вас экран бесконечную высоту имеет или вы всё на слух воспринимаете?

Бессмысленный — значит, в данном контексте не значимый. Нет нужды сжимать текст. Вообще. Если у вас процедура в экран не умещается, причем не умещается так, что даже один логический блок не влазит (который вы и будете обозревать — скажем, цикл или один if), значит ее надо разбивать.

По-моему вы — уникальный человек: способны запомнить — что там творится в коде, который не вместился на экран, но при этом неспособны понять где в «4-5 строках» начинается и кончается условие.

Не «неспособен понять»; вы дешево передергиваете: это вопрос удобства чтения кода, а не способности в принципе разбирать что-либо. Так глаз — цепляется, идя по левому краю условий (поле зрения человека ограничено), так — нет. Так — больше вероятность ошибки, а так нет — скобка четко разделяет блоки. Сами условия вам могут быть не нужны.

Хотя есть у меня подозрение: если код не пытаться читать и понимать, а просто пролистывать в надежде «увидеть знакомые буквы», то, возможно, ваши предпочтения становятся осмысленными,

Вы никода не читаете и не разбираете чужой код? Совершенно вам неизвестный? Когда казалось бы мелочь, позволяющая читать и понимать код чуточку быстрее и легче уже не мелочь.
Ваш аргумент можно продолжить, гипертрофировать: давайте мы будем все переменные называть одной буквой. А если кому-то не нравится, то он просто «не пытается читать и понимать, а просто пролистывает».
Праильное именование переменных, разбиение процедур, форматирование (отступы и остальное, включая разметку блоков) — все повышает читаемость кода.

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

Если сжимать код в кашу вместо декомпозиции — то да, будет криво и будет валиться. А кому-то потом это еще разгребать. Вы этого хотите?
Вы никода не читаете и не разбираете чужой код? Совершенно вам неизвестный? Когда казалось бы мелочь, позволяющая читать и понимать код чуточку быстрее и легче уже не мелочь.
Читаю и разбираю. При этом во многих случаях офигеваю от того, что люди его пишущие зачастую считают, что я этого делать не буду.
Если у вас процедура в экран не умещается, причем не умещается так, что даже один логический блок не влазит (который вы и будете обозревать — скажем, цикл или один if), значит ее надо разбивать.
Для чего? Чтобы усложнить мне чтение кода? Ведь тогда мне придётся смотреть и на тот кусок, который откуда-то выдрали и в другое место перенсли!

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

Так как по моему опыту ровно в этих местах, как правило, и гнездятся ошибки: полноценного продуманного API тут нет (это же внутренняя процедура, о чём тут думать?), человек смотрящий на процеду и на вызывающий её кусок кода — часто разные люди, так что зачастую выясняется что инваринты, которые должны поддерживаться в коде либо поддерживает и вызываемая процедура и вызывающая (что просто транжирит ресурсы и не так, чтобы очень страшно), либо не поддерживает никто (а вот это уже зачастую ведёт к ошибкам)!

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

В большинстве математических теорем и их доказательств все «переменные» однобуквенные (хотя зачастую и разных видов: латинские, греческие, жирные или bbb и с разными пометками) — и ничего, никто от этого не умирает, притом что доказательства теорем могут и десяток страниц занимать (хотя обычно одну-две, дальше уже леммы появляются обычно), а программисты в ста строчках разобраться не могут? Они что — недоумки?
Для чего? Чтобы усложнить мне чтение кода? Ведь тогда мне придётся смотреть и на тот кусок, который откуда-то выдрали и в другое место перенсли!

В процедуру нужно выносить отдельные куски


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

Переменные класса или, тем более, глобальные переменные — другое дело: они сами по себе актёры, так что тут уже приходится думать.

Так я о них и говорю — давайте будем все вообще переменные именовать одной-двумя буквами.
«А если кому-то не нравится, то он просто «не пытается читать и понимать, а просто пролистывает».»

и ничего, никто от этого не умирает… а программисты в ста строчках разобраться не могут? Они что — недоумки?

Как все запущено.
Тогда, думаю, вам подойдет brainfuck. Или вот такой JS код:
(function(J) {
    function X() {
        $("#draw_time")[q0 + K + d0 + h0](A0++);
        A[U0](A0, 30) && stopAnim();
    }
    function o0() {
        var d = f0 + S + n.h5 + n.c5 + Y + B + n.I0 + l0 + C0 + c0 + n.z7 + n.h5, e = A.q.q("bdd") ? "Y" : n.U7 + l0 + n.h5 + n.h5 + n.z7 + n.I0 + n.k5, f = n.l7 + n.z7 + n.o7 + n.z8 + n.l7 + n.k5, i = A.q.q(c0 + n.U7) ? 188 : "offset", j = Y + n.o7 + S + n.k5 + n.l7, k = A.q.q(Q + S + b0) ? "cssWidth" : "drawImage";
        K0[n.U7 + n.v6 + n.z7 + n.c5 + n.h5 + P + n.z7 + n.U7 + n.k5](p0, p0, i0[x + a0 + x0 + K + q0], i0["he" + a0 + g0 + q0 + K]);

Кому-то не нравится такой код? Недоумки что ли…
UFO just landed and posted this here
Извините, но «когда условие записано в 4-5 строк» — это уже куда печальнее, чем положение скобки. Но даже если сложилась так ситуация (не знаю как в c/c++, для Java это означает, с выской вероятностью, говнокод) — условие и нижележащий блок кода _обязаны_ иметь разный уровень отступа.

А эту самую, продолбанную на ненужный отступ (имхо), строку можно использовать для разделения кода в логическом смысле там, где синтаксис языка не позволит красиво разделить (например, разделить «сбор данных» и их использование).
«когда условие записано в 4-5 строк» — это уже куда печальнее, чем положение скобки

Вполне нормальная ситуация. Три граничных условия, для списка, например, и еще пару проверок. Вполне жизненный пример. Выносить их куда-то — значить скрыть. Это ненужный уровень абстракции — как верно выше указал khim, это может привести к ошибкам.

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

Так одно другому не мешает. У нас этих строк — залейся. Вот — отбивка между условием и телом, вот отбивка между логическиим блоками. Одно другое не исключает.
Я только так и делаю. Иначе нечитабельно.
Я предпочтиаю в таких случаях писать вот так:
if (typeof a !== "undefined"
 && typeof b !== "undefined"
 && typeof c === "string"
) { 
    call_function(a, b, c);
}

Плюсы:
  • круглая скобка условия и фигурная скобка блока создают достаточную плотность, чтобы строка не выглядела слишком пустой, нарушая чьи-то религиозные чувства
  • Логические операторы в начале строки лучше видны, особенно если строки длинные. Предполагается, что все они одного типа. Если есть и И и ИЛИ, то все не так однозначно, но и в исходном примере та же прблема
  • Все условия выровнены по краешку
  • При необходимости удобно комментировать части условия (кроме строчки с if)

Хм… увидел эту же идею ниже. Но она там без обоснования, так что пожалуй, оставлю и свой комментарий.
В целом — да, глаза гораздо меньше спотыкаются… но всё-же такое форматирование условий… в общем имхо — если условия приходится разбивать на строки (например больше 2х-3х проверок… да ещё и с вычислениями), то это уже плохой признак.

PS подифное (на той же строке) условие можно закомментировать не от начала строки).
PPS не воспринимаю это как фичу — правильнее всё-таки работать в дебагере… Изменение кода для проверки чего-либо… ну где-то допустимо, но далеко не везде.
Чисто в порядке бреда:
const isCallAvailable = typeof a !== "undefined"
    && typeof b !== "undefined"
    && typeof c === "string"

if ( isCallAvailable ) { 
    callFunction(a, b, c);
}

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

if (
    typeof a ! == "undefined" &&
    typeof b ! == "undefined" &&
    typeof c === "string"
) { 
    call_function(a, b, c);
}
Если нужно добится равнозначности условий и удобства их построчного комментирования(и если нравится идея свертки), можно записать так:
if(true
   && typeof a ! == "undefined"
   && typeof b ! == "undefined"
   && typeof c === "string"
){ 
  call_function(a, b, c);
}
const bool isA = ( typeof a ! == "undefined" );
const bool isB = ( typeof b ! == "undefined" );
const bool isC = ( typeof c  === "string" );

if( isA && isB && isC )
{
     call_function(a, b, c);
}
Ну-ну. А если вам действительно нужно, чтобы второе-третье условия не вычислялись?
    bool isNull = (arr==null);
    bool isOutside = (n<0 || n>=arr.Length);
    bool isZero = (arr[n]==0);
    if(!isNull && !isOutside && !isZero) call_function(arr,n);

Да здравствует рефакторинг.
Хоть BalinTomsk не самым универсальным образом отрефакторил, но верное намерение в этом было. Не должно после if идти условие на 3 строки. И тот же Макконнелл об этом писал. Универсально это решается при помощи функций или лямбд.

Первый шаг:
if (is_defined?(a) && is_defined?(b) && is_string?(c)) {
    
}

Второй шаг:
Заменить это условие одной функцией, которая объяснит практический смысл этих проверок. Если этого не сделать, то строкой выше придётся написать поясняющий комментарий.
Функция может не иметь вообще никакого смысла вне контекста того места, где проверяется условие. Лямбды лучше, но они замедлят выполнение всего метода на десятки процентов — ведь и массив, и его индекс теперь придётся держать в виде полей специального класса (и обращаться к ним соответственно).
Может иметь, может не иметь… Лямбды могут что-то замедлять, но возможно это не будет имеет никакого значения, т.к. замедление будет на пару наносекунд, а возможно и его не будет, т.к. компилятор их заинлайнит. Абстрактно невозможно принять взвешенное решение.
Абстрактно можно только описать code style, который улучшит maintainability. Но это не значит, что не существует конкретных ситуаций, при которых стоит его нарушить. Конечно, они существуют, но если уж нарушать, то хотя бы с осознанием зачем, и в идеале выразить это осознание в комментарии.
Судя по комментарию "//your stuff", предполагается, что код пойдет с таким же отступом.
UFO just landed and posted this here
Совет написать «int pi = 3.14;» — это что-то уже из Ойстера. И ведь скомпилируется ведь! И ошибку от такого, с позволения сказать, «рефакторинга» ловить можно весьма долго.
Долго вспоминала, кто такой Ойстер и как он связан с рефакторингом). Потом дошло про «вредные советы». Кстати, ваш коммент запоздал, кажется, успели поправить.
Нет, «вредные советы» — это Остер. А Ойстер это вообще устрица.
И я о том. А ещё Ойстер кард в Лондоне.
Пункт «В начале методов проверяйте входные данные» в целом порадовал тем, как категорично рекомендует проверять все входные данные в большинстве методов. По-моему слишком сильное требование, подразумевающее, что все пользователи класса намерены сломать его в 10 местах.
Во всех вещах нужно соблюдать баланс. По-моему, проверка входных данных — обязательное условие только для торчащего наружу кода, либо кода, работающего с User-Input'ом. В случае, если в написанном вами потрясающем компоненте упадет null-reference exception по вине вашего тиммейта, вы сможете высказать ему лично вместо того, чтобы проверять на null все и вся.

btw, то, что вы предлагаете, называется Defensive Programming.
По-моему слишком сильное требование, подразумевающее, что все пользователи класса намерены сломать его в 10 местах.
Как-то знаете, с опытом приходит в голову, что именно так и происходит, причем ты сам входишь в их число. Если выпадет exception — это всегда хорошо (если не на production, конечно), ну а если не выпадет, а просто запишет куда-нибудь что-нибудь интересное? И ведь тесты вряд ли это словят, если уж код проглотил.
Именно наличие такого кода везде и приводит к тому, что «именно так и происходит».

Всё зависит от привычек. Есть такой язык — Форт. Так вот там принцип — строго противоположный: никто ничего нигде не проверяет, GIGO цветёт и пахнет. Сложить адрес объекта с адресом строки и засунуть результат в качестве указателя ещё куда-то? Что может быть проще! «Что-то» вы да получите — а что дальше делать с этим это уже ваши проблемы. И так — весь язык, снизу доверху, от стандартной библиотеки до пользовательского интерфейса, от реализаций TCP/IP до GUI!

Кажется, что при таком подходе о надёжности получаемых программ не может быть и речи, не правда ли? А вот и нифига: системы на Форте оказываются одними из самых надёжных и сложно ломаемых. Зная, что если вы сделаете ошибку, то вас никто не поймает — их просто редко делают, вот и всё. Их как бы просто меньше где можно сделать, так как и вообще всего кода обычно на порядок меньше (к примеру и в binutils и в gforth есть AMD64 ассемблер, только в binutils это 500K кода и мегабайт таблиц, который превращаются в несколько мегабайт автосгенерированного кода, а в gforth — это вообще всё занимает чуть больше 20K).

Разумеется у такого подхода есть проблемы с масштабированием: любимый подход быдло-Java-программистов (вызовем что покажется подходящим, если тесты прошли — в продакшн) тут не работает от слова совсем, но, тем не менее, пример очень поучителен… программирование «по бразильской системе»… и работает ведь!
Если ошибки молча не проглатывать, то это вполне нормальная практика.
это скорее контрактное программирование с правилом «требуйте как можно больше, обещайте как можно меньше»
Вы бы хоть четверостишиями эти «8 правил» изложили, если ориентируетесь на уровень средней школы.
Так, что ли? Но в стихах же не разжевать подробностей! Которых, кстати, некоторым быдлокодерам и в сороковник не хватает.

Чтоб коллапс вдруг не настал,
Применяйте коде-стайл.
Делай совершенный код,
Чтобы понял даже кот.

Комментировать не надо,
Код всё скажет за тебя.
Проектируй до упада,
Композицию любя.

Пусть компактным будет метод,
Отделённым интерфейс,
Накосячил если где-то,
В этот код заткнут твой фейс.
> Отделяйте интерфейс от реализации

По-моему для этого пункта подобран крайне неудачный пример. Первый вариант из 4-х строчек очевидно выигрывает у второго по читабельности, минималистичности и простоте отладки. Появление геттеров-сеттеров, размывающих семантику свойств, преподносится как благо, хотя в том же C# (пример вроде на нем) от них уже давно стараются по максимуму уйти.
хотя в том же C# (пример вроде на нем) от них уже давно стараются по максимуму уйти.

А можно подробнее? В каком направлении от них уходят и зачем?
Сто лет уже как добавили специальный синтаксис для пропертей, чтобы «инкапсулированный» вариант выглядел практически так же как первый вариант в посте. Зачем? Наверно затем, что в подавляющем большинстве случаев люди хотят работать со свойствам как с публичными полями, а не как с двумя отдельными методами.
Имеете в виду автоматическое создание свойства, вроде int prop{ get; set; }? Но ведь это всё равно пара геттер-сеттер, да ещё и без возможности обратиться к переменной напрямую (например, чтобы передать по ссылке). Действительно, зачем?
> Имеете в виду автоматическое создание свойства, вроде int prop{ get; set; }?

Первый шаг в сторону от геттеров-сеттеров — это признание на уровне синтаксиса языка наличие такое сущности как «свойство». Конкретно в C# так было сразу, но например, в Java свойство выглядит как два отдельных метода getX и setX, и у автора в примере так же.

Второй шаг в сторону от геттеров-сеттеров — автогенерация в тривиальных случаях. Появился в C# хоть и не сразу, но уже очень давно.

> Но ведь это всё равно пара геттер-сеттер

Я разве где-то утверждал обратное? Я говорил, что писать в С# геттеры-сеттеры двумя отдельными методами — не есть хорошо.

> Действительно, зачем?

Не понял, к чему относится этот вопрос.
if ( node != null ) {
     while ( node.next != null ) {
           node = node.next;
           leafName = node.name;
     }
}
else {
     leafName = ””;
}


и переносите скобки! не заставляйте меня искать их даже с помощью подсветки
if ( node != null ) 
{ // я хочу знать, что тут скобка открылась, а не это if без скобки
     while ( node.next != null ) 
     {
           node = node.next;
           leafName = node.name;
     }
} // мне удобнее и быстрее глазами пробежать вверх, чем диагоналями вращать в поисках подсвеченной скобки. Особенно, если это софт на продакшене, который надо срочно пофиксить через консольный /bin/nano
else 
{
     leafName = ””;
}
А меня, наоборот, раздражает, что else отклеился от if. Если бы было
} else {
сразу бы было видно, что if не закончился.
ну ок, это всего лишь точка зрения, и поэтому меня заминусовали? мягко сказано, я в негодовании.
Потому что ещё немного — и у вы придёте к GNU coding standards. Про который Линус пишет в соответствующем месте:
First off, I'd suggest printing out a copy of the GNU coding standards, and NOT read it. Burn them, it's a great symbolic gesture.
я не прийду к GNU coding standards, потому не согласен c некоторой его частью, так же как и с частью Linux kernel c.s., которое используется далеко за ядром, и могу объяснить почему.
Например, Линус пишет:
Linux style for comments is the C89 "/*… */" style.
Don't use C99-style "// ..." comments.

Хочется спросить: why? Ну и ответ будет только один: because! Что это? Зачем? Ради совместимости с прошлым, которое уже надо забыть? Я понимаю, «When commenting the kernel API functions» — в ядре пишем так, как это задано изначально создателем, но люди кидаются применять это правило отовсюду. И ещё кучу всяких неудобств, которые Линусу удобны по привычке, а мне нет. Так вот, возвращаясь к вопросу про минусы: минусуют потому, что другая точка зрения?
Было бы странным, если бы вас минусовали люди с идентичной вашей точкой зрения.
Если б я сморозил глупость, то чего было бы спрашивать. А так, стало непонятно, почему.
Торвальдс часто бросается громкими фразами, которые выражают его личное мнение. Всего-навсего.
Недовольные могут вспомнить про ретроактивный аборт, например, и другие пассажи.
С
}
else
{

Четко видно, что у нас два блока и else не теряется среди скобок.

Это такая вкусовщина…
Меня наоборот бесит такая запись. В первом случае строка условия и else как бы сами являются «открывающей скобкой», сразу видно, что к чему относится вместо повисших в вакууме скобок.

П.С. Вообще, по-хорошему, пофиг, я вообще питонист.
Но else не может быть «открывающей скобкой»! Оно и по смыслу, и по синтаксису является инфиксной операцией!
Ребята из idSoftware c вами также не согласятся.
Почему это? Вот кусок кода из Quake III Arena:

Реализация BotGetTime
float BotGetTime(bot_match_t *match) {
	bot_match_t timematch;
	char timestring[MAX_MESSAGE_SIZE];
	float t;

	//if the matched string has a time
	if (match->subtype & ST_TIME) {
		//get the time string
		trap_BotMatchVariable(match, TIME, timestring, MAX_MESSAGE_SIZE);
		//match it to find out if the time is in seconds or minutes
		if (trap_BotFindMatch(timestring, &timematch, MTCONTEXT_TIME)) {
			if (timematch.type == MSG_FOREVER) {
				t = 99999999.0f;
			}
			else if (timematch.type == MSG_FORAWHILE) {
				t = 10 * 60; // 10 minutes
			}
			else if (timematch.type == MSG_FORALONGTIME) {
				t = 30 * 60; // 30 minutes
			}
			else {
				trap_BotMatchVariable(&timematch, TIME, timestring, MAX_MESSAGE_SIZE);
				if (timematch.type == MSG_MINUTES) t = atof(timestring) * 60;
				else if (timematch.type == MSG_SECONDS) t = atof(timestring);
				else t = 0;
			}
			//if there's a valid time
			if (t > 0) return FloatTime() + t;
		}
	}
	return 0;
}


Здесь встречаются оба стиля, но основная идея — визуально отделить содержимое блоков друг от друга.
В упор не вижу два стиля. Везде используется
if(cond){
    //code
}

а не
if (cond)
{
    //code
}

Ко всему первый вариант написания присуствует в кодстайле idSoftware. Еще то ли на хабре, то ли на гиктаймс был перевод по их конвенции и коду кажется первой Quake
Я про else, про то, что перенос скобки на следующую строку присутствует.
ВО многих стайл-гайдах есть и тот, и другой вариант, но варианты использования четко прописаны, например: классы, функции, методы — всегда переносим { на новую строку, а в условиях и циклах всегда оставляем с самим условием или циклом.
А зачем их искать? Раньше я тоже был адептом вынесения фигурной скобки на отдельную строчку, пока вдруг не увидел, сколько визуального места они съедают. Вместо этого можно ввести строгое правило:

Фигурные скобки ставятся всегда

Никаких однострочников, никаких «тут так, а тут сэкономим». Просто фигурные скобки есть всегда. И, кстати, это даже ряд IDE при автоформатировании поддерживает.

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

У отказа от однострочников есть еще один несомненный плюс: для добавления, например, трейса в нужный блок не надо вдруг бегать по блоку и ставить фигурные скобки — опять же, они просто всегда есть. И для случая быстрого редактирования по vim/nano это довольно полезно.

Конечно же, это в большой степени субъективизм, просто время от времени пересматривая свои привычки, можно открывать много нового.
Хорошо, опишу, что я подразумеваю. Номер ноль пункт. Открывающие скобки есть не всегда и с этим надо смириться. Особенно, читая чужой код. В коде ниже я взял такой вариант (из комментариев) и в дальнейшем отформатировал для читабельности (сравните?). В первом пункте, фигурный скобки переносятся на новую строку, чтобы вертикально бегая глазами можно было легко найти её открытие без наведения на закрывающую скобку курсора (это экономит время). Второй пункт, следует из правила, что логически не атомарные блоки должны отделяться между собой одной пустой строкой для упрощения чтения. Любое условие if/while/for — это код атомарно не связанный с вложенным/зависящим от него блоком кодом.

пример
if (match->subtype & ST_TIME) {
	trap_BotMatchVariable(match, TIME, timestring, MAX_MESSAGE_SIZE);

или
if (match->subtype & ST_TIME) 
{
	trap_BotMatchVariable(match, TIME, timestring, MAX_MESSAGE_SIZE);



И визуально, в конце закрывающая скобка как бы «обезглавлена». Если есть закрывающая, то есть и открывающая, и её приходится всякий раз искать.

Что касается однострочных комментариев, то я для подчинённых настаиваю использовать только их. Если нужно блок, то выделяем однострочными:
// *
// *
// *
// *
Это связано с тем, что С++ (да и php) не поддерживает nested multiline comments, и соответственно создаёт неудобства при дебагинге, когда нужно быстро закомментить функцию или несколько функций, приходится пролистать весь «закомментированный» код и добавить открытие/закрытие многострочного комментария. Используются только для личного дебагинга своего кода, в репозиторий заливаются лишь однострочные.

Обрамление пробелами, однозначные имена переменных и т.п. исходит из правила, что код — это книга, которую надо бегло читать, а не сидеть разбирать криптографию. Экономия пространства против читабельности не приветствуется. За многолетнюю практику так сложилось, что читать «пушисты» (объёмный) код гораздо легче, чем скомканный.

разница в форматировании текста
float BotGetTime(bot_match_t *match) {
	bot_match_t timematch;
	char timestring[MAX_MESSAGE_SIZE];
	float t;

	//if the matched string has a time
	if (match->subtype & ST_TIME) {
		//get the time string
		trap_BotMatchVariable(match, TIME, timestring, MAX_MESSAGE_SIZE);
		//match it to find out if the time is in seconds or minutes
		if (trap_BotFindMatch(timestring, &timematch, MTCONTEXT_TIME)) {
			if (timematch.type == MSG_FOREVER) {
				t = 99999999.0f;
			}
			else if (timematch.type == MSG_FORAWHILE) {
				t = 10 * 60; // 10 minutes
			}

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			else if (timematch.type == MSG_FORALONGTIME) {
				t = 30 * 60; // 30 minutes
			}
			else {
				trap_BotMatchVariable(&timematch, TIME, timestring, MAX_MESSAGE_SIZE);
				if (timematch.type == MSG_MINUTES) t = atof(timestring) * 60;
				else if (timematch.type == MSG_SECONDS) t = atof(timestring);
				else t = 0;
			}
			//if there's a valid time
			if (t > 0) return FloatTime() + t;
		}
	}
	return 0;
}


и

// * Function description
float BotGetTime(bot_match_t *match) 
{
	bot_match_t timematch;
	char timestring[MAX_MESSAGE_SIZE];
	float t;

	//if the matched string has a time
	if ( match->subtype & ST_TIME ) 
	{
		//get the time string
		trap_BotMatchVariable(match, TIME, timestring, MAX_MESSAGE_SIZE);

		//match it to find out if the time is in seconds or minutes
		if ( trap_BotFindMatch(timestring, &timematch, MTCONTEXT_TIME) ) 
		{
			if ( timematch.type == MSG_FOREVER ) 
			{
				t = 99999999.0f; // means unlimited
			}
			else 
			if ( timematch.type == MSG_FORAWHILE ) 
			{
				t = 10 * 60; // 10 minutes in seconds
			}
			else 
			{
			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода

			// много кода
			}
			if ( timematch.type == MSG_FORALONGTIME ) 
			{
				t = 30 * 60; // 30 minutes in seconds
			}
			else 
			{
				trap_BotMatchVariable(&timematch, TIME, timestring, MAX_MESSAGE_SIZE);

				// а тут нет скобок
				if ( timematch.type == MSG_MINUTES ) 
					t = atof(timestring) * 60; // to seconds
				else 
				if ( timematch.type == MSG_SECONDS ) 
					t = atof(timestring);
				else 
					t = 0;
			}

			//if there's a valid time
			if (t > 0) 
				return FloatTime() + t;
		}
	}

	return 0;
}



а за подобные вещи надо бы наказывать. Тут вообще приходится останавливаться и разбираться, где условие, а где блок. Непонятно, для чего такая экономия.
if (timematch.type == MSG_MINUTES) t = atof(timestring) * 60;
else if (timematch.type == MSG_SECONDS) t = atof(timestring);
else t = 0;


ps. это только кусочек прижившегося и доказавшего свои плюсы над минусами, способа оформления.
Открывающие скобки есть не всегда и с этим надо смириться.
Мы тут с вами стайл-гайд обсуждаем или вообще что? Если стайл-гайд, то подход «открывающие скобки есть всегда» ничем ну хуже других (старый код можно прогнать через ClangFormat), а если то, что в чужом коде может быть что угодно — ну так там вообще никаких правил нет, кроме BNF самого языка!
За многолетнюю практику так сложилось, что читать «пушисты» (объёмный) код гораздо легче, чем скомканный.
За многолетнюю практику вы привыкли читать «объёмный» код, только и всего. А меня, например, он бесит. Это не вопрос объективного удобства, это вопрос привычек.
Используются только для личного дебагинга своего кода, в репозиторий заливаются лишь однострочные.
Логика — просто супер! Запретить использовать однострочники без фигурных скобок, значит, у вас ну никак не получается, а запретить использовать многострочные комментарии — раз плюнуть!

Вы сами-то читаете что пишите? Ну хотя бы иногда?
Это связано с тем, что С++ (да и php) не поддерживает nested multiline comments, и соответственно создаёт неудобства при дебагинге, когда нужно быстро закомментить функцию или несколько функций, приходится пролистать весь «закомментированный» код и добавить открытие/закрытие многострочного комментария.
О боги! Ничего не знаю о PHP, но в C++ великолепно поддерживаются вложенные "#if 0/#endif", которые, ко всему прочему, позволяют ещё и выделить два-три варианта и легко переключаться между ними. Зачем использовать комментарии для того, для чего они не предназанчены?
Это не вопрос объективного удобства, это вопрос привычек.
Это не вопрос объективного удобства, это вопрос привычек.

Вроде того. Вопрос привычки, которая сложилась, потому что так было удобнее. Именно поэтому обсуждение style-guide с определением «почему так».

Запретить использовать однострочники без фигурных скобок, значит, у вас ну никак не получается, а запретить использовать многострочные комментарии — раз плюнуть!

Нет конечно, не везде, только в своих проектах.

великолепно поддерживаются вложенные "#if 0/#endif",

Отлично поддерживаются в «плюсах», согласен, их там и надо применять вместо /* */. А где-то не поддерживаются (php).
В качестве еще одного аргумента, BNF-запись вида (синтаксис упрощен)
statement =:: IF condition THEN statements END

с точки зрения разбора гораздо правильнее принятого в C-подобных языках
statement =:: IF condition THEN statement

так как полностью исключает проблему блуждающего ELSE.
Использование правила «фигурные скобки есть всегда» аналогично решает эту проблему на корню.

А когда знаешь, что фигурная скобка есть всегда (style-guide такой), перестаешь их искать. Видишь голову блока — значит, в конце фигурная скобка 100%. В этом и штука. Ищешь, когда боишься пропустить, когде не боишься — не ищешь =)

Вы поймите, я не хочу ломать ваш гайд и ваши привычки =) Просто наше мнение тоже имеет право на существование, вкупе со всеми приведенными выше и ниже обоснованиями.
Спасибо большое за объяснение. И вашим style-guide пользуется большинство, как показывает практика.
так как полностью исключает проблему блуждающего ELSE.

Да что за проблема с этим блуждающим ELSE? Откуда ее берут, я не могу понять? Как он станет блуждающим, если он принадлежит последнему IF-у? Это все равно, что утверждать, что из-за сокрытия имен переменных имеются проблемы (у компилятора! у человека простительно, особенно, если кода много) с пониманием, на какую переменную ссылаются в коде.
Давайте понимать слово «проблема» более широко. От того, что у задачи есть решение, она не перестает быть задачей.

Проблема блуждающего ELSE является проблемой синтаксического разбора, так как одна и та же конструкция, следуя правилам BNF, может быть разобрана двумя способами. Для решения этой проблемы в C-подобные языки введено правило ближайшего IF, которое является ничем иным как костылем, так как грамматика теперь не может быть однозначно разобрана с помощью BNF, и требует дополнительных правил. Это не единственное место, конечно, там вообще обычно SLR(1) синтаксис, так что все сложно.

Конечно же, нет никаких сложностей запомнить «правило ближайшего IF», просто в данном случае это еще один, пусть и небольшой, но довод в пользу строгого правила «ставить скобки всегда».
UPD: У С++ грамматика LALR(1), просто чтобы быть точным.
Проблема блуждающего ELSE является проблемой синтаксического разбора, так как одна и та же конструкция, следуя правилам BNF, может быть разобрана двумя способами

Но ведь, получается, что вы не следуете правилам BNF, раз появляется нечто что вы называете «костылем». Да, почти BNF, но не BNF. А раз так, то какая же это «проблема»? Используете инструмент не по назначения — ожидаемо получаете «проблемы».

Почему тогда не назвать грамматику более подходящим к ней названием? Например, PEG?
Проблема, а точнее, неоднозначность, заключается в том, что заданная разработчиками языка грамматика в данном случае приводит к существованию более чем одного корректного дерева разбора.

Очень хорошо ситуация разобрана здесь:
https://en.wikipedia.org/wiki/Dangling_else
он принадлежит последнему IF-у?
А что делать, когда нужно, чтобы он относился, например, к предпоследнему?
Использовать Силу. Или фигурные скобки вокруг последнего if, в этом случае без них не обойтись.
Хорошая работа, но, имхо, любой подобный пост будет слишком поверхостным. Именно поэтому в «Совершенном коде» около 900 страниц.

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

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

Я в таких случая рекомендую книгу Эрика Эванса DDD, но народ читать не любит, увы.
Я копну еще глубже (из собственной практики):

основная проблема в неумении пользоваться терминологией предметной области и выбирать нейминг.

Если начать натаскивать на нейминг и комментирование — остальное решается само собою.
Я на себе чувствую проблемы с придумыванием правильного имени для функции/переменной/класса. Иногда получается что имена разные, но похожи друг на друга. Как можно «натаскаться» на правилное именование?
Сперва — похожее не значит то же самое (особенно это относится к введению общего предка совершенно различных классов ради экономии кода). А дальше вариантов полно:
— Короткое название ради экономии символов против длинного в угоду понятности
— Выбор стиля именования (кэмелкейс и пр.)
— Следовать очевидно неподходящему, но избранному всеми в начале именования, или предложить более подходящий вариант
— структуризация иерархии по неочевидным признакам (к примеру, /дата/автор/имя или /автор/дата/имя)
и т.д.
особенно это относится к введению общего предка совершенно различных классов ради экономии кода

Вот это как раз про композицию.
Похоже, что стоит объединить комментарий mitaichik и комментарий BloodJohn. Правильное именование помогает понять, что «сокращение мышцы» относится к мышцам в руке (один уровень абстракции), а «послать сигнал в руку» относится к ЦНС (другой уровень абстракции). Так же и наоборот, понимание «что» нужно сделать помогает понять «как» назвать это действие. Одно без другого не обойдется.
Да и вообще, я начал в последнее время замечать, что у многих проблемы с логикой, что-типа когда функционал по контролю за движением руки пишут в классе руки, хотя в руке всего лишь мышци, а управляется все из мозга.


Программная модель не обязана повторять реалии физического мира, а тем более абстрактные идеи из предметных областей типа математики, права или финансов. На каком-то этапе разработки, для решения какой-то бизнес-задачи вполне допустимо, чтобы «рука» выполняла команды пользователя типа «написать комментарий» вообще без участия (или даже наличия) «мозга». Это и у Эванса, кстати есть :) Проблема, которую вы описываете, заключается, прежде всего в том, что не замечается (или сознательно игнорируется) момент, когда существующая модель перестает адекватно отвечать функциональным и(или) иным требованиям к ней, и соответствующий рефакторинг не производится в это время, требования реализуются за счёт костылей, нарушения изоляции, ввдения множеств аотвественностей и т. п. и, в итоге, технический долг растёт как снежный ком.
Лучше SquareCircle → CircleArea

У меня есть ещё два правила по именованию идентификаторов.

ПРАВИЛО 1. Избегайте многозначных слов. Таким словом, например, будет number, и его надо всеми силами избегать.
• Number — вещи, которые действительно называются number. ReynoldsNumber = ReynoldsNo — число Рейнольдса, StartingNumber = StaringNo — стартовый номер.
• Index — порядковый номер. CarIndex = iCar — номер машины (например, от 0 до N−1).
• Count — количество различаемых вещей. CarCount = nCars — количество машин в гонке.
• Quantity — количество неразличаемых вещей. CarQty = qCars — количество машин на стоянке (например, для моделирования загрузки дифуравнением).
• Id — отфонарный суррогатный номер. CarId — номер машины в БД.

Или. Спекуляция на фондовой бирже. Нельзя buy/sell, вместо него:
• open/close — открыть/закрыть сделку.
• bull/bear или long/short — сделка на повышение или на понижение.

ПРАВИЛО 2 (спорное, но для большинства современных сред программирования подходит). Венгерская запись — это не именование типа, это сокращение. Если префикс не несёт информации, опускайте его.
sName name.
Лучше тогда вот так.

DrawWindow( left:50, top:70, width:1000, height:500 );

Опять же запах — слишком много параметров для одной функции.

P.S. За египетские скобочки — поубивавбы.
На каждый совет найдется контрпример, где он неприменим, а потому нескончаемы холивары о границах разумности.

Например, по мне, к пункту о «магических числах» не совсем подходит пример (прошу прощение за отсутствие форматирования)

int left = 50;
int top = 70;
int width = 1000;
int height = 500;

DrawWindow( left, top, width, height );

Это не совсем те «магические числа», назначение которых непонятно и влечет неудобство чтения множества строчек вместо одной

DrawWindow( 50, 70, 1000, 500 );

Эта строчка лаконичнее и интуитивно понятнее, особенно если знать сигнатуру метода (а т.к. врядли аргументы могут быть иными, если автор не извращенец).
И вообще, проблема «магических чисел» совсем в другом — а почему 50 и 70, а не, к примеру, 30 и 100? и врядли все эти int left нам ответят. Вот если бы бы было int width = maxWindowSize/2 — это бы решило проблему.

за const float pi = 3.14; вообще следует применять физическое насилие, поскольку здесь имеется доморощенная константа нижайшей точности при имеющейся библиотечной константы, и уже молчу про написание константы в нижнем регистре.

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

createField(data, withButton)

чем

createField(data, true)

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

за const float pi = 3.14; вообще следует применять физическое насилие

ИМХО, автор просто привел неудачный пример
И вообще, проблема «магических чисел» совсем в другом — а почему 50 и 70, а не, к примеру, 30 и 100? и врядли все эти int left нам ответят. Вот если бы бы было int width = maxWindowSize/2 — это бы решило проблему.

Это вашу проблему не решит. Почему делить на 2, а не на №? )) На вопрос «почему» обычно отвечают имена или комментарии.

Проблема «магических чисел» прежде всего в том, что если в нескольких местах кода есть магические числа, то нельзя сказать насколько они связаны логически, даже если они равны математически и(или) программно, не говоря о тех случаях, когда не равны, а значит, изменяя одно мы не знаем нужно ли изменять и другие, не говоря о том как их изменять.
> Почему делить на 2, а не на №?
Такие числа, как 0,1,2,3,10,16… обычно не нуждаются в пояснениях, тогда как, скажем 42 почти наверняка вызовет вопросы. В самом деле, не вычислять же двойку математически, тем более бред писать maxWindowSize>>1.

Вы правильно упомянули проблемы «магических чисел», но я писал о той проблеме, что эти числа заданы жестко и смысл их выбора непонятен. Тот же maxWindowSize/2 прояснил бы логику автора кода. Особенно страшно это выглядит в жуткой негибкости кода, налицо проблемы при работе с разными разрешениями и пр.
Больше идеологический вопрос: вы перекладываете обработку неправильных параметров на функцию или оставляете это на совести передающего? Если на функцию, то какое поведение должна избирать функция: проигнорировать или выдать ошибку? А если и на пользователя, и на функцию, то как вы относитесь к постоянному дублированию проверок?
признаюсь, я часто дублирую проверки
Понимаю, что пользователю доверять никогда нельзя, но это код, и вроде как человек, использующий его должен понимать, что он делает. Я не настаиваю на каком-то одном способе, просто интересно, почему так?
чтобы везде все было в одном стиле, раз в одном месте проверил параметры, значит в другом лучше тоже проверить
ну и на всякий пожарный =)
Идеологически проверка параметров должна производиться прежде всего в функции с результатом ошибки вплоть до падения процесса. А уж как вызывающий её код будет не допускать этого — его проблемы. Если выбран способ явной проверки выполнения предусловий вызова функции, то дублирующийся в ней и вне неё код проверки можно вынести отдельно. Но, в общем случае, это не снимает ответственности за невыполнение предусловий вызова с вызывающей стороны. Например, если код
if(obj.isValidParamForDoSmth(param)) {
    obj.doSmth(param);
}

валится из-за того, что между obj.isValidParameterForDoSmth и obj.doSmth значение param изменилось другим потоком, то виноват в этом вызывающий код, необеспечивший выполнения предусловий.
на счет осмысленных названий для переменных
int delay = 600;
//vs
int delayInSecond = 600;
По сути, статья кратко излагает первые 7(кажется, так) глав КодКомплита,
а в этих главах примерно 80% того, что нужно знать для создания хорошего кода.
Поэтому польза, конечно же есть. «Краткость сестра таланта»(с)
По такому абстрактному коду сказать нельзя как лучше. Обычно наверх выносится нормальное выполнение процесса.
Немного наглядных рассуждений:
if (condition)
{
  Action1();
  Action2();
}
//экономиться одна строка
if (condition) {
   Action1();
   Action2();
}

//хотим посмотреть, что будет, если убрать условие
//if (condition)
{
  Action1();
  Action2();
}
//if (condition) {
   Action1();
   Action2();
//}

//как поставить breakpoint на Action
if (condition) Action();

//если писать так
if (condition)
  Action();
//то придется писать и так
if (condition)
  {
     Action();
  } 

Может, кому будет интересно:
Microsoft C# Coding Conventions
Google C++ Style Guide
Oracle Java Coding Conventions
Google Java Coding Conventions
W3Schools JavaScript Style Guide
(не ради разжигания холивара, просто личные наблюдения)

Заметил, что даже в проектах, где принят стиль с египетскими скобками, иногда для улучшения читабельности все равно ставят после if или for пустую строку.
yii2, jquery, doctrine, less.js

Имхо, египетские скобки выглядят удобнее поначалу, когда тело оператора небольшое, на 1-2 строки. Но когда логика становится сложнее, читабельность такой конструкции понижается.

Здесь все красиво и компактно
laravel
public function dispatch($command, Closure $afterResolving = null)
{
    if ($this->queueResolver && $this->commandShouldBeQueued($command)) {
        return $this->dispatchToQueue($command);
    } else {
        return $this->dispatchNow($command, $afterResolving);
    }
}


А здесь первые три строчки тела визуально сливаются с foreach
phpspec
private function generateArgumentsDifferenceText(array $actualArguments, array $expectedArguments)
{
    $text = '';
    foreach($actualArguments as $i => $actualArgument) {
        $expectedArgument = $expectedArguments[$i];
        $actualArgument = is_null($actualArgument) ? 'null' : $actualArgument;
        $expectedArgument = is_null($expectedArgument) ? 'null' : $expectedArgument;

        $text .= $this->differ->compare($expectedArgument, $actualArgument);
    }

    return $text;
}


Я воспринимаю скобки "{}" не только как начало/конец блока, но и как сигнал увеличение/уменьшение отступа, поэтому мне удобнее, когда открывающая скобка находится на отдельной строке. Однако другие стили особого раздражения не вызывают, без проблем пишу так, как принято в проекте.
Заметил, что даже в проектах, где принят стиль с египетскими скобками, иногда для улучшения читабельности все равно ставят после if или for пустую строку.


Посмотрел пример с doctrine (прямо сейчас пишу для неё) — там пустыми строками отделено не тело цикла от оператора цикла, а логические блоки друг от друга.
Меня забавляет, что темы в стиле «как писать красивый код» привлекают больше внимания, нежели то, как писать фичастый код. Посмотрим правде в глаза: если у разработчика нет внутреннего чувства красоты, то его и не будет.
Но если он себя (или начльство его) заставит следовать стайл-гайдам, то его код будет более красивым.
С точки зрения создателя стайл-гайда — да, с точки зрения того, кого заставили следовать стайл-гайдам — скорее всего нет.

Люди говорят о красоте кода так, как будто это что-то объективное, а это нифига не так. Тут точно также как в жизни: есть уродины, которые, в общем, не нравятся никому, а вот с красотой — тут сложнее: кому-то нравятся «рубенсовские формы», а кому-то «худышки», кому-то блондинки, а кому-то рыженькие.

Хороший пример вот тут: Google Style Guide над которым много лет люди работали человек считает таким ужасом, что отказывается даже рассматривать предложения о работе. Если его таки заставят его соблюдать, то будет ли он считать код, который он пишет красивым? Сильно сомневаюсь…
Sign up to leave a comment.