Академия плохого кода: переводы строк, пробелы и отступы

Привет, Хабр! Представляю вашему вниманию перевод статьи «Dark code-style academy: line breaks, spacing, and indentation» автора zhikin2207

image

Привет, народ! Позвольте мне продолжить рассказ про нашу академию плохого кода. В этом посте мы раскроем другой путь замедления чтения вашего кода. Следующие приёмы помогут вам уменьшить понимание вашего кода и увеличить шансы на появление в нём багов. Готовы? Давайте начнём.

Переводы строк, пробелы и отступы могут убивать.


Как люди читают книги? Сверху вниз, слева направо (по крайней мере — большинство). Это же происходит, когда разработчики читают код. Одна строка кода должна содержать одну мысль, следовательно, каждая строка должна содержать только одну команду. Если вы хотите смутить других разработчиков, вам лучше нарушить эти принципы. И давайте я покажу вам как это сделать.

Пример №1

Посмотрите на этот кусок кода. Одна идея на одной строке. Код такой чистый, что меня аж тошнит.

return elements
    .Where(element => !element.Disabled)
    .OrderBy(element => element.UpdatedAt)
    .GroupBy(element => element.Type)
    .Select(@group => @group.First());

Мы можем объединить все операторы в одну строку, но это было бы слишком просто. В этом случае мозг разработчика поймёт, что здесь что-то не так, и он разобьёт операторы слева направо. Проще простого!

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

return elements.Where(e => !e.Disabled)
    .OrderBy(e => e.UpdatedAt).GroupBy(e => e.Type)
    .Select(g => g.First());

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

return elements.Where(e => !e.Disabled)
               .OrderBy(e => e.UpdatedAt).GroupBy(e => e.Type)
               .Select(g => g.First());

Отправьте мне открытку, если этот подход пройдёт код-ревью в вашей команде.

Совет: оставьте пару операторов на одной строке и пару — на отдельных строках.

Пример №2

Абсолютно та же идея здесь. Только такой код вы видите намного чаще.

var result = 
    (condition1 && condition2) || 
    condition3 || 
    (condition4 && condition5);

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

var result = (condition1 && condition2) || condition3 || 
    (condition4 && condition5);

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

var result = (condition1 && condition2) || condition3 || 
             (condition4 && condition5);

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

Совет: играйте с переводами строк для получения лучшего результата.

Пример №3

Что насчёт этого?

if (isValid) 
{ 
    _unitOfWork.Save();
    return true; 
} 
else 
{ 
    return false; 
} 

Та же проблема, но — с другой стороны. Здесь лучшим вариантом будет объединить операторы в одну строку, конечно, расставив фигурные скобки.

if (isValid) { _unitOfWork.Save(); return true; } else { return false; } 

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

Совет: объединяйте маленькие if/for/foreach операторы в одну строку.

Пример №4

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

bool IsProductValid(
    ComplexProduct complexProduct, 
    bool hasAllRequiredElements, 
    ValidationSettings validationSettings)
{
    // code
}

Проще всего замедлить чтение вашего кода, заставив других разработчиков скроллить ваш код горизонтально. Просто игнорируйте правило 80 символов.

bool IsProductValid(ComplexProduct complexProduct, bool hasAllRequiredElements, ValidationSettings validationSettings)
{
    // code
}

Это супер просто: забыть, что было до того, как вы начали скроллить, или пропустить строку, на которой вы начали. Отличный трюк.

Совет: нарочно игнорируйте правило 80 символов.

Пример №5

Пустая строка в правильном месте – это сильный инструмент для группировки вашего кода и ускорения его чтения.

ValidateAndThrow(product);

product.UpdatedBy = _currentUser;
product.UpdatedAt = DateTime.UtcNow;
product.DisplayStatus = DisplayStatus.New;

_unitOfWork.Products.Add(product);
_unitOfWork.Save();

return product.Key;

Пустая строка в неправильном месте в сочетании с другими советами этой статьи может помочь вам сохранить вашу работу. Какую пустую строку вы предпочитаете?

ValidateAndThrow(product);
product.UpdatedBy = _currentUser;
product.UpdatedAt = DateTime.UtcNow;

product.DisplayStatus = DisplayStatus.New;
_unitOfWork.Products.Add(product);

_unitOfWork.Save();
return product.Key;

Совет: вставляйте пустые строки рандомно.

Пример №6

Когда вы делаете коммит в репозиторий, у вас есть крошечная возможность посмотреть, что именно вы собираетесь коммитить. НЕ ДЕЛАЙТЕ ЭТОГО! Это нормально, если вы добавили лишнюю пустую строку как здесь.

private Product Get(string key) 
{
    // code
}

private void Save(Product product) 
{
    // code
}

Или, что еще лучше, — добавили несколько пробелов на пустой строке (чтобы понять разницу, выделите 5ю строку).

private Product Get(string key) 
{
    // code
}
    
private void Save(Product product) 
{
    // code
}

Зачем вам это нужно? Код продолжает работать (но это не точно). Вы продолжаете понимать ваш код, но другой разработчик будет понимать ваш код меньше. Вы не можете просто добавить несколько лишних пробелов в общие методы сразу (код-ревью – наш враг), но использование этой практики создаст беспорядок уже через пару недель активной разработки.

Ещё одна дополнительная выгода при использовании лишних пробелов в строке появляется тогда, когда другие разработчики коммитят связанную функциональность, их IDE может автоматически исправлять форматирование. На код-ревью они увидят тысячу красных и зелёных строк. Если вы понимаете о чём я ;)

По этой же причине вы можете настроить табы в вашей IDE, если вы используете пробелы в вашем проекте, и наоборот.

Совет: не смотрите на код перед коммитом.

Пример №7

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

product.Name = model.Name;
product.Price = model.Price;
product.Count =  model.Count;

Совет: знай своего врага.

Трудная работа – сделать свой код неподдерживаемым. Когда вы копите много мелких проблем, они растут без вашего участия. Молодые разработчики будут писать свой код по вашим шаблонам. В один прекрасный день в течение код-ревью вы услышите «Что за фигня?» от вашего тимлида, и тут вы получите возможность использовать коронную фразу: «Что? Мы всегда так делаем», и покажете ему тысячу мест в коде, где написано так же.

Развлекайтесь.

Комментарии 55

    +7
    тогда другие разработчики будут форматировать ваш код десятилетиями

    Один хоткей в решарпере, он по дефолту именно так и выравнивает linq по точкам.


    добавили несколько пробелов на пустой строке (чтобы понять разницу, выделите 5ю строку)

    То же самое и здесь: тот же хоткей, который просто въедается в автоматизм.


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

      +3

      Хоткей не нужен. Автоформатирование должно происходить при сохранении.

        +2

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


        Ещё весело бывает, когда кто-то заботливо добавлял пробелы чтобы получить табличное форматирование, а потом пришла IDE, и при сохранении автоматически всё испортила.

          +9
          Конфиг автоформаттера должен хранится в той же самой репе.
          У нас, например, .clang-format лежит в корне.
            +2

            Откуда они там встречаются? Ревьювер должен дать по шапке тому, кто отключил гитхуки при коммите и залил фигню.


            Все настройки форматирования и линтеров в проекте лежать должны.

              0

              В идеале код вообще может автоформатироваться под каждого разработчика.

                0
                В идеале прямо в глазах. А пока такого нет, лучше содержать код в одном стиле, чтобы при ревью или парном программировании никого не смущать.
                +2
                Ещё весело бывает, когда кто-то заботливо добавлял пробелы чтобы получить табличное форматирование

                Зачем это делать? Нет, оно конечно выглядит красиво, не спорю, но любое изменение длины строк приводит к переформатированию подобного кода и куче изменённых строк в системе контроля версий.
                Нет, так делать не нужно.
                  0

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

                    +1
                    С чего бы это?
                      0

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

                        0
                        Ну формула, ну и что? Если там есть присваивание переменной с по сути произвольным именем, то опять таки получится фигня.
                        Обычно я вижу такое форматирование в местах подобным этому:
                        $var     = $x * 2;
                        $varName = $x * 3;
                        

                        И если тут переименовать вторую переменную, то придётся править и первую строку. А таких присвоений может быть десяток.
                        А про какие случаи вы говорите?
                          +1
                          $x2 =  $x1 * cos($angle) + $y1 * sin($angle);
                          $y2 = -$x1 * sin($angle) + $y1 * cos($angle);
                            0
                            Ну, я в своей работе с подобным не сталкиваюсь. Ну разве что.
            +3

            Redundand else детектед:


            if (isValid) 
            { 
                _unitOfWork.Save();
                return true; 
            } 
            else 
            { 
                return false; 
            } 

            Лучше, т.к. экономит горизонтальное пространство для блока else:


            if (isValid) 
            { 
                _unitOfWork.Save();
                return true; 
            }
            
            return false; 
            
              +18
              Тогда уж:
              if (isValid) 
              { 
                  _unitOfWork.Save();
              }
              
              return isValid; 
                +1

                Хороший вариант, особенно когда есть понимание, что ничего больше в блоках добавляться не будет. Версия которую я предложил — та что решарпер подсказывает с включенной Redundand else

                  +1
                  Это в любом случае хороший вариант. А если в будущем в блоках нужно будет что-то добавлять — то тогда в будущем и будет изменена схема с if-else-return блоками.
                  –5

                  Должны быть так
                  return (isValid)? _unitOfWork.Save(): false;
                  Функция должна вернуть результат своей работы. Иначе это лотерея.

                    0
                    Не делайте так. Даже асм читабельнее.
                    Впрочем, C++ уверенно развивает традиции.
                      +1
                      return isValid? _unitOfWork.Save(),true: false;
                        –1
                        return isValid? _unitOfWork.Save(),isValid: isValid;

                        :)
                          0
                          return (isValid? _unitOfWork.Save(): (void)0), isValid;
                +13
                80 символов в строке – это рекомендуемый стандарт на текущий момент.

                На какой момент? На 1985г?
                Лично у меня влезает 360 символов на экран. Открываю 2-3 исходника.

                var result =
                (condition1 && condition2) ||
                condition3 ||
                (condition4 && condition5);

                Предполагается что это хороший стиль? Это нечитабельное месиво.
                Напротив, «неправильный» пример аккуратный и читабельный.

                  +9
                  Лично у меня влезает 360 символов на экран.
                  Во-первых, это лично у вас. Во-вторых, влезать может хоть 500, читать-то это как?
                    –4

                    На телефоне не влезает 360. А 80 норм.

                      +8

                      Для мержа придется найти экран на 720 символов.

                        +3

                        (сарказм)


                        Легко! Берете 2 экрана и сравниваете


                        Но пока я не придумал, что делать с разработчиками, у которых 2 экрана, поэтому они пишут код в 720 символов сразу
                        Всё-таки 4 монитора для этого понадобится


                        (сарказм офф)


                        Мне кажется основная проблема длинного текста не в том, что он на экран не влезает, а в том, что куча всего смешивается в одну строку и это становится трудночитаемым. Например, пример 2 из статьи, если это всё смешать в одну строчку, читать становится гораздо труднее (даже если это поместится на экран)

                          +1
                          Можно просто уменьшить шрифт в 2 раза! Можно прямо в статью вынести.
                            0

                            Надеюсь это была шутка…

                          0
                          Не отстаивая именно 80, замечу, что при мерже на экране бывают сразу три версии файла. И три по 80 — это уже 240. А так мне тоже кажется, что при нынешних мониторах допустимо поднять это ограничение до 120 символов.
                            0
                            сейчас некоторые переходят на 100 и 120
                            +17
                            80 символов в строке – это рекомендуемый стандарт на текущий момент. Это позволяет вам держать концентрацию разработчика, когда он читает ваш код. Более того, вы можете открыть два документа одновременно на одном экране когда это нужно, и у вас останется место для обозревателя решений.

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

                              +1
                              Более того, даже в таком консервативном проекте, как ядро Linux, все-таки отказались от ограничения в 80 символов в code style:
                              lkml.iu.edu/hypermail/linux/kernel/2005.3/08168.html
                              git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46
                                +3

                                линус — мужик!
                                но я так так и не понял, с какого потолка они взяли цифру 100.

                                  0

                                  Я думаю — просто магия ровных цифр, так я видел и 100 и 120. Много уже проектов в кодстайле прописали 100 символов, мммм… навскидку: https://github.com/airbnb/javascript

                                    +8
                                    Я думаю — просто магия ровных цифр

                                    для программистов ровная цифра — это 128 :)


                                    https://github.com/airbnb/javascript

                                    какой веселый кодстайл, не рекомендует начинать имена свойств с подчеркивания

                                      0

                                      127. потому что за ней может идти -128.

                              +2
                              Вы им только не говорите, что сейчас можно еще подключать несколько мониторов.
                                +3

                                Это не времена DOS-а. Это гораздо древнее. Ещё перфокарты были по 80 колонок. Этакое физическое ограничение длины строки.

                                +10
                                Пример №2 лучше писать так:
                                var result = 
                                    (condition1 && condition2)
                                    || condition3
                                    || (condition4 && condition5)
                                ;
                                

                                Оно и читается легко и при изменении в diff попадёт адекватный лог. Добавив или удалив условие мы в diff-е получим меньше мусора.
                                  –2

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

                                    0
                                    С этим вариантом одно плохо: не для всех языков подходит(замечу, что тэга С/С++ у статьи нет, значит она про все языки). А именно для языков, которые считают, что точка с запятой не нужна. Вот, скажем, для Go такой вариант будет как раз вредным советом, ибо компилятор тут же выкинет нечто вроде(Go не понимает тут, где строка кончается):

                                    ./prog.go:10:10: syntax error: unexpected ||, expecting }
                                    +3

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

                                      –4
                                      deleted
                                        +6
                                        var result = 
                                            (condition1 && condition2) || 
                                            condition3 || 
                                            (condition4 && condition5);

                                        я бы разбил так:


                                        var result =  condition3 
                                                  || (condition1 && condition2) 
                                                  || (condition4 && condition5);

                                        Логический оператор в начале строки лучше читается

                                          +1

                                          Только вот переставлять условия местами я бы не стал:


                                          var result = (condition1 && condition2)
                                               || condition3
                                               || (condition4 && condition5);

                                          Вдруг это хитрые свойства с побочными действиями?

                                            0

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

                                              +1

                                              В performance-critical коде первыми поставили бы наиболее быстро проверяемые условия (например, проверить булевские значения), а в конец — более медленные (например, сравнение строк), так что универсального подхода тут нет.

                                              +2
                                              Переставлять условия вообще нельзя: если значение логического выражения определяется уже по значению первого операнда (true для ||, false для &&), то второй операнд не вычисляется вообще (по крайней мере, в C#, на котором написаны примеры, это однозначно так). То есть, действует правило неполного вычисления, и в C# им пользуются весьма часто — например, сначала идет проверка ссылки на объект на !=null, а потом, через && — проверка значения поля или свойства (да, про операцию ?. я в курсе, но она появилась не так давно, много кода было написано до того).
                                              0
                                              Для Go только первый вариант рабочий. Так что, сначала надо определиться с языком, а потом уже переформатировать :)
                                              +3
                                              Ещё один вредный совет:
                                              Если вы пишете под Android, используйте m для префиксов членов класса, например mIsValid, ведь сразу становится понятно, что это field, а не variable /s
                                              На самом деле, это «m» — просто жесть. Пошло оно с тех времён, когда core команда не могла нормально использовать IDE, поскольку Eclipse не осиливал загрузить такой большой объём кода в себя. Потом пришла IDEA/Android Studio, которая уже нормально осиливала, но ретрограды из гугла, которые могут в уме инвертировать бинарное дерево не осилили уже саму IDE, и поэтому пишут вот в таком стиле до сих пор.
                                              А самое печальное — что они же пишут доки, откуда этот «m» разносится по проектам 3rd-party разработчиков, которые понятия не имеют, зачем оно нужно, но тупо копипастят, потому что ж «гугол решил, а там не дураки работают».

                                              P.S. Те, кому до сих пор кажется, что «там не дураки работают» — посмотрите внимательно код AOSP Launcher, такое ощущение, что туда как в Спарте сбрасывают самых слабых разработчиков. А это, по сути, Core Android.
                                                +1

                                                Это и других языков касается. Меня очень увидило, когда я установил Rider, и он начал ругаться, что у меня имена полей не соответствуют кодстайлу по умолчанию. По его мнению, все поля должны начинаться с "_". Зачем?

                                                  0
                                                  Способ форматирования фигурных скобок в определениях функций, который мне очень нравится, но я никогда его не использую.
                                                  Smth foo(int par)
                                                  {   some code
                                                      more code
                                                      return smth;
                                                  }
                                                  Smth bar(int par)
                                                  {   some code
                                                      more code
                                                      return smth;
                                                  }
                                                  

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

                                                  Самое читаемое