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

В комментариях к нашему первому посту на Хабре пользователи активно обсуждали каналы восприятия информации. Мы подумали и решили, что тему «совершенного кода» стоит развить и изложить ещё и письменно — ведь базовые принципы хорошего кода едины для программистов, пишущих на любом языке.

Зачем нужен хороший код, когда всё и так работает?


Несмотря на то, что программа исполняется машиной, программный код пишется людьми и для людей — неслучайно высокоуровневые языки программирования имеют человекопонятные синтаксис и команды. Современные программные проекты разрабатываются группами программистов, порой разделённых не только офисным пространством, но и материками и океанами. Благо, уровень развития технологий позволяет использовать навыки лучших разработчиков, вне зависимости от места нахождения их работодателей. Такой подход к разработке предъявляет серьёзные требования к качеству кода, в частности, к его читабельности и понятности.

Существует множество известных подходов к критериям качества кода, о которых рано или поздно узнаёт практически любой разработчик. Например, есть программисты, которые придерживаются принципа проектирования KISS (Keep It Simple, Stupid! — Делай это проще, тупица!). Этот метод разработки вполне справедлив и заслуживает уважения, к тому же отражает универсальное правило хорошего кода — простоту и ясность. Однако простота должна иметь границы — порядок в программе и читабельность кода не должны быть результатом упрощения. Кроме простоты, существует ещё несколько несложных правил. И они решают ряд задач.

  • Обеспечивать лёгкое покрытие кода тестами и отладку. Unit тестирование — это процесс тестирования модулей, то есть функций и классов, являющихся частью программы. Создавая программу, разработчик должен учитывать возможности тестирования с самого начала работы над написанием кода.

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

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

  • Упрощать процесс внесения дальнейших изменений. Чем лучше оптимизирована структура, тем проще изменять код, добавлять новые свойства, повышать быстродействие и изменять архитектуру.

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

  • Обеспечивать возможность поддержки проекта несколькими разработчиками или целыми сообществами (особенно важно для проектов с открытым исходным кодом).

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

8 правил хорошего кода по версии GeekBrains


Соблюдайте единый Code style. Если программист приходит работать в организацию, особенно крупную, то чаще всего его знакомят с правилами оформления кода в конкретном проекте (соглашение по code style). Это не случайный каприз работодателя, а свидетельство серьёзного подхода.

Вот несколько общих правил, с которыми вы можете столкнуться:

соблюдайте переносы фигурных скобок и отступы — это значительно улучшает восприятие отдельных блоков кода
соблюдайте правило вертикали — части одного запроса или условия должны находиться на одном отступе
if (typeof a ! == "undefined" &&
    typeof b ! == "undefined" &&
    typeof c === "string") { 
//your stuff
}

соблюдайте разрядку — ставьте пробелы там, где они улучшают читабельность кода; особенно это важно в составных условиях, например, условиях цикла.
for (var i = 0; i < 100; i++) {
}

В некоторых средах разработки можно изначально задать правила форматирования кода, загрузив настройки отдельным файлом (доступно в Visual Studio). Таким образом, у всех программистов проекта автоматически получается однотипный код, что значительно улучшает восприятие. Известно, что достаточно трудно переучиваться после долгих лет практики и привыкать к новым правилам. Однако в любой компании code style — это закон, которому нужно следовать неукоснительно.

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

Например, в коде есть строка:
DrawWindow( 50, 70, 1000, 500 );

Очевидно, она не вызовет ошибок в работе кода, но и её смысл не всем понятен. Гораздо лучше не полениться и сразу написать таким образом:
int left = 50;
int top = 70;
int width = 1000;
int height = 500;

DrawWindow( left, top, width, height );

Иногда магические числа возникают при использовании общепринятых констант, например, при записи числа ?. Допустим, в проекте было внесено:
SquareCircle = 3.14*rad*rad 

Что тут плохого? А плохое есть. Например, если в ходе работы понадобится сделать расчёт с высокой точностью, придётся искать все вхождения константы в коде, а это трата трудового ресурса. Поэтому лучше записать так:
const float pi = 3.14;
SquareCircle = pi*rad*rad 

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

Используйте осмысленные имена для переменных, функций, классов. Всем программистам известен термин “обфускация кода” — сознательное запутывание программного года с помощью приложения-обфускатора. Она делается с целью скрыть реализацию и превращает код в невнятный набор символов, переименовывает переменные, меняет имена методов, функций и проч… К сожалению, случается так, что код и без обфускации выглядит запутанно — именно за счёт бессмысленных имён переменных и функций: var_3698, myBestClass, NewMethodFinal и т.д… Это не только мешает разработчикам, которые участвуют в проекте, но и приводит к бесконечному количеству комментариев. Между тем, переименовав функцию, можно избавиться от комментариев — её имя будет само говорить о том, что она делает.

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

Например, возьмём код такого вида:
// находим лист, записываем в r 
if ( x != null ) {
    while ( x.a != null ) {
          x = x.a; 
          r = x.n;
    } 
} 
else { 
    r = ””; 
}

Из комментария должно быть понятно, что именно делает код. Но совершенно неясно, что обозначают x.a и x.n. Попробуем внести изменения таким образом:
// находим лист, записываем имя в leafName
if ( node != null ) {
     while ( node.next != null ) {
           node = node.next;
           leafName = node.name;
     }
}
else {
     leafName = ””;
}

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



Объединим в одно разъяснение ещё два важных правила. Создавайте методы как новый уровень абстракции с осмысленными именами и делайте методы компактными. Вообще, сегодня модульность кода доступна каждому программисту, а это значит, что нужно стремиться создавать абстракции там, где это возможно. Абстракция — это один из способов сокрытия деталей реализации функциональности. Создавая отдельные небольшие методы, программист получает хороший код, разделённый на блоки, в которых содержится реализация каждой из функций. При таком подходе нередко увеличивается количество строк кода. Есть даже определённые рекомендации, которые указывают длину метода не более 10 строк. Конечно, размер каждого метода остаётся целиком на усмотрении разработчика и зависит от многих факторов. Наш совет: всё просто, делайте метод компактным так, чтобы один метод выполнял одну задачу. Отдельные вынесенные сущности проще улучшить, например, вставить проверку входных данных прямо в начале метода.

Для иллюстрации этих правил возьмём пример из предыдущего пункта и создадим метод, код которого не требует комментариев:
string GetLeafName ( Node node ) {
      if ( node != null ) {
 	   while ( node.next != null ) {
 	       node = node.next;
           }
           return node.name;
      } 
 return ””;
}

И, наконец, скроем реализацию метода:
... leafName = GetLeafName( node ); …

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

В примере ниже вставляем проверку на то, чтобы на входе не получить null.
List<int> GetEvenItems( List<int> items ) {
 	Assert( items != null);

	 List<int> result = new List<int>();
	 foreach ( int i in items ) {
		 if ( i % 2 == 0 ) {
 			result.add(i);
 		}
	 }
	 return result;
}

Реализуйте при помощи наследования только отношение «является». В остальных случаях – композиция. Композиция является одним из ключевых паттернов, нацеленных на облегчение восприятия кода и, в отличие от наследования, не нарушает принцип инкапсуляции. Допустим, у вас есть класс Руль и класс Колесо. Класс Автомобиль можно реализовать как наследник класса-предка Руль, но ведь Автомобилю нужны и свойства класса Колесо.

Соответственно, программист начинает плодить наследование. А ведь даже с точки зрения обывательской логики класс Автомобиль — это композиция элементов. Допустим, есть такой код, когда новый класс создаётся с использованием наследования (класс ScreenElement наследует поля и методы класса Coordinate и расширяет этот класс):
сlass Coordinate {
 public int x;
 public int y;
}
class ScreenElement : Coordinate {
public char symbol;
}

Используем композицию:
сlass Coordinate {
 public int x;
 public int y;
}
class ScreenElement {
 public Coordinate coordinate;
 public char symbol;
}

Композиция — неплохая замена наследованию, этот паттерн более простой для дальнейшего понимания написанного кода. Можно придерживаться такого правила: выбирать наследование, только если нужный класс схож с классом-предком и не будет использовать методы других классов. К тому же, композиция избавляет программиста ещё от одной проблемы — исключает конфликт имён, который случается при наследовании. Есть у композиции и недостаток: размножение количества объектов может оказывать влияние на производительность. Но опять же, это зависит от масштаба проекта и должно оцениваться разработчиком в каждом случае отдельно.

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

Возьмём класс:
class Square {
 public float edge;
 public float area;
}

Ещё один класс:
class Square {
 public float GetEdge();
 public float GetArea();
 public void SetEdge( float e );
 public void SetArea( float a );
 private float edge;
 private float area;
}

Второй случай предпочтительнее, так как он скрывает реализацию с помощью модификатора доступа private. Кроме улучшения читабельности кода, отделение интерфейса от реализации в сочетании с соблюдением правила создания небольшого интерфейса даёт ещё одно важное преимущество: в случае нарушений в работе программы для поиска причины сбоя потребуется проверить лишь несколько функций. Чем больше открытых функций и данных — тем сложнее отследить источник ошибки. Однако интерфейс должен быть полным и должен позволять делать всё, что необходимо, иначе он бесполезен.

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

Для тех, кто воспринимает информацию через видео и хочет услышать детали вебинара — видеоверсия:



Другие наши вебинары можно посмотреть здесь.

Комментарии (111)


  1. zo_oz
    02.11.2015 18:57
    +13

    Беру первый же пример

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

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


    Как с читаемостью? Легко видите где кончается условие издалека? :)


    1. Subrisk
      02.11.2015 19:20

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


    1. khim
      02.11.2015 19:35
      -2

      А нефиг устраивать отсутпы в 4 символа.

      if (typeof a ! == "undefined" &&
          typeof b ! == "undefined" &&
          typeof c === "string") { 
        call_function(a, b, c);
      }
      
      Никаких проблем с читабельностью.


      1. Wedmer
        02.11.2015 19:57
        +8

        А если у него отступы делаются табуляциями? Вообще любой нормальный редактор при длине отступа в 4 символа сделает так:

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

        Фактически автоформатирование нарушает это правило.


        1. romy4
          02.11.2015 21:14
          +9

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

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


          1. webkumo
            02.11.2015 23:32
            +1

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


            1. ZakharS
              03.11.2015 08:55
              +4

              А для меня как раз удивительно, когда открывающую скобку оставляют в конце строки. Дело привычки.
              У нас в компании код-стандарт как раз такой, как в комментарии у romy4


            1. MacIn
              03.11.2015 19:19

              Конечно, удобно. Сразу видно, где закончилось условие, и где началось тело. Недели за 2 взгляд привыкает, проверено.


              1. webkumo
                04.11.2015 00:14
                +1

                Так где условие кончается — и так видно (многоуровневое условие должно иметь другой уровень отступа, нежели код в блоке условия), а вот бесполезно истраченная строка… меня вот смущает.


                1. MacIn
                  04.11.2015 00:30
                  -1

                  Так где условие кончается — и так видно

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

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

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

                  По-моему, это бессмысленный термин, извините.


                  1. khim
                    04.11.2015 01:16
                    +1

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

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


                    1. MacIn
                      04.11.2015 01:46

                      Что значит «бессмысленный»? У вас экран бесконечную высоту имеет или вы всё на слух воспринимаете?

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

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

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

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

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

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

                      Если сжимать код в кашу вместо декомпозиции — то да, будет криво и будет валиться. А кому-то потом это еще разгребать. Вы этого хотите?


                      1. khim
                        04.11.2015 16:25

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

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

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

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

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


                        1. MacIn
                          04.11.2015 23:14

                          Для чего? Чтобы усложнить мне чтение кода? Ведь тогда мне придётся смотреть и на тот кусок, который откуда-то выдрали и в другое место перенсли!

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


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

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

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

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

                          Как все запущено.
                          Тогда, думаю, вам подойдет 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]);
                          

                          Кому-то не нравится такой код? Недоумки что ли…


                    1. Mixim333
                      04.11.2015 09:37

                      Не знаю как Вы, но лично я также оператор begin (символ '{') также всегда переношу на новую строчку — мне так и читать удобнее и правки вносить. Яркий, хотя и надуманный пример с отступами: программист случайно или в силу каких-то проблем с клавиатурой не нажал Tab и '{' оставил на той же строке, что и условие — как читать будете?

                      Плюс, лично у меня есть привычка, возможно дурацкая, все условные операторы разделять скобками и переносами на отдельные строки я не занимаюсь, т.е. не: «if(a==b&&b>c)», а: «if( (a==b) && (b>c) )» — так и при рефакторинге лучше и отлаживать проще.


                  1. webkumo
                    04.11.2015 11:20

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

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


                    1. MacIn
                      04.11.2015 23:17

                      «когда условие записано в 4-5 строк» — это уже куда печальнее, чем положение скобки

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

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

                      Так одно другому не мешает. У нас этих строк — залейся. Вот — отбивка между условием и телом, вот отбивка между логическиим блоками. Одно другое не исключает.


            1. lucid
              06.11.2015 07:28

              Я только так и делаю. Иначе нечитабельно.


            1. Mingun
              06.11.2015 19:31
              +2

              Я предпочтиаю в таких случаях писать вот так:

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

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

              Хм… увидел эту же идею ниже. Но она там без обоснования, так что пожалуй, оставлю и свой комментарий.


              1. webkumo
                06.11.2015 22:50

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

                PS подифное (на той же строке) условие можно закомментировать не от начала строки).
                PPS не воспринимаю это как фичу — правильнее всё-таки работать в дебагере… Изменение кода для проверки чего-либо… ну где-то допустимо, но далеко не везде.


              1. Jofr
                06.11.2015 22:55

                Чисто в порядке бреда:

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

                Естественно, имя переменной должно быть чуть более осмысленным :)


                1. VolCh
                  08.11.2015 14:38
                  +1

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


        1. EvilPartisan
          02.11.2015 23:21

          Удалено


        1. lampslave
          03.11.2015 02:11
          +14

          Ещё вариант.

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


          1. dtestyk
            04.11.2015 01:54
            +2

            Если нужно добится равнозначности условий и удобства их построчного комментирования(и если нравится идея свертки), можно записать так:

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


      1. BalinTomsk
        02.11.2015 21:23
        +5

        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);
        }
        


        1. Mrrl
          02.11.2015 21:48
          +8

          Ну-ну. А если вам действительно нужно, чтобы второе-третье условия не вычислялись?

              bool isNull = (arr==null);
              bool isOutside = (n<0 || n>=arr.Length);
              bool isZero = (arr[n]==0);
              if(!isNull && !isOutside && !isZero) call_function(arr,n);
          

          Да здравствует рефакторинг.


          1. Source
            02.11.2015 23:15
            +1

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

            Первый шаг:

            if (is_defined?(a) && is_defined?(b) && is_string?(c)) {
                
            }
            

            Второй шаг:
            Заменить это условие одной функцией, которая объяснит практический смысл этих проверок. Если этого не сделать, то строкой выше придётся написать поясняющий комментарий.


            1. Mrrl
              03.11.2015 07:16

              Функция может не иметь вообще никакого смысла вне контекста того места, где проверяется условие. Лямбды лучше, но они замедлят выполнение всего метода на десятки процентов — ведь и массив, и его индекс теперь придётся держать в виде полей специального класса (и обращаться к ним соответственно).


              1. Source
                07.11.2015 18:02

                Может иметь, может не иметь… Лямбды могут что-то замедлять, но возможно это не будет имеет никакого значения, т.к. замедление будет на пару наносекунд, а возможно и его не будет, т.к. компилятор их заинлайнит. Абстрактно невозможно принять взвешенное решение.
                Абстрактно можно только описать code style, который улучшит maintainability. Но это не значит, что не существует конкретных ситуаций, при которых стоит его нарушить. Конечно, они существуют, но если уж нарушать, то хотя бы с осознанием зачем, и в идеале выразить это осознание в комментарии.


    1. Wedmer
      02.11.2015 19:50

      Судя по комментарию "//your stuff", предполагается, что код пойдет с таким же отступом.


    1. stychos
      05.11.2015 13:36

      Я всегда так пишу:

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

      тогда видно и связующие части условия


  1. khim
    02.11.2015 19:31
    +6

    Совет написать «int pi = 3.14;» — это что-то уже из Ойстера. И ведь скомпилируется ведь! И ошибку от такого, с позволения сказать, «рефакторинга» ловить можно весьма долго.


    1. Itimora
      02.11.2015 19:44
      +1

      Долго вспоминала, кто такой Ойстер и как он связан с рефакторингом). Потом дошло про «вредные советы». Кстати, ваш коммент запоздал, кажется, успели поправить.


      1. Mrrl
        02.11.2015 20:06
        +6

        Нет, «вредные советы» — это Остер. А Ойстер это вообще устрица.


        1. Itimora
          02.11.2015 20:57

          И я о том. А ещё Ойстер кард в Лондоне.


  1. mokonaDesu
    02.11.2015 20:07
    +7

    Пункт «В начале методов проверяйте входные данные» в целом порадовал тем, как категорично рекомендует проверять все входные данные в большинстве методов. По-моему слишком сильное требование, подразумевающее, что все пользователи класса намерены сломать его в 10 местах.
    Во всех вещах нужно соблюдать баланс. По-моему, проверка входных данных — обязательное условие только для торчащего наружу кода, либо кода, работающего с User-Input'ом. В случае, если в написанном вами потрясающем компоненте упадет null-reference exception по вине вашего тиммейта, вы сможете высказать ему лично вместо того, чтобы проверять на null все и вся.

    btw, то, что вы предлагаете, называется Defensive Programming.


    1. yul
      02.11.2015 21:30

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


      1. khim
        02.11.2015 22:20
        +2

        Именно наличие такого кода везде и приводит к тому, что «именно так и происходит».

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

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

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


    1. alexeibs
      02.11.2015 21:56

      Если ошибки молча не проглатывать, то это вполне нормальная практика.


    1. Borz
      03.11.2015 00:56

      это скорее контрактное программирование с правилом «требуйте как можно больше, обещайте как можно меньше»


  1. OlegMax
    02.11.2015 20:42
    +1

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


    1. Subrisk
      02.11.2015 21:34
      +31

      Так, что ли? Но в стихах же не разжевать подробностей! Которых, кстати, некоторым быдлокодерам и в сороковник не хватает.

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

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

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


  1. nehaev
    02.11.2015 20:56

    > Отделяйте интерфейс от реализации

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


    1. Mrrl
      02.11.2015 21:00

      хотя в том же C# (пример вроде на нем) от них уже давно стараются по максимуму уйти.

      А можно подробнее? В каком направлении от них уходят и зачем?


      1. nehaev
        02.11.2015 21:10
        +1

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


        1. Mrrl
          02.11.2015 21:44

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


          1. nehaev
            02.11.2015 22:45

            > Имеете в виду автоматическое создание свойства, вроде int prop{ get; set; }?

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

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

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

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

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

            Не понял, к чему относится этот вопрос.


  1. romy4
    02.11.2015 21:09
    -10

    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 = ””;
    }
    


    1. Mrrl
      02.11.2015 21:50
      +17

      А меня, наоборот, раздражает, что else отклеился от if. Если бы было
      } else {
      сразу бы было видно, что if не закончился.


      1. romy4
        03.11.2015 12:56

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


        1. khim
          03.11.2015 15:08

          Потому что ещё немного — и у вы придёте к 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.


          1. romy4
            03.11.2015 16:13

            я не прийду к 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» — в ядре пишем так, как это задано изначально создателем, но люди кидаются применять это правило отовсюду. И ещё кучу всяких неудобств, которые Линусу удобны по привычке, а мне нет. Так вот, возвращаясь к вопросу про минусы: минусуют потому, что другая точка зрения?


            1. Wedmer
              03.11.2015 18:25
              +3

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


              1. romy4
                03.11.2015 18:39

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


          1. MacIn
            03.11.2015 19:25

            Торвальдс часто бросается громкими фразами, которые выражают его личное мнение. Всего-навсего.


            1. MacIn
              03.11.2015 22:01

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


      1. MacIn
        03.11.2015 19:26

        С
        }
        else
        {

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

        Это такая вкусовщина…


    1. Goobs
      02.11.2015 22:16
      +12

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

      П.С. Вообще, по-хорошему, пофиг, я вообще питонист.


      1. Mrrl
        03.11.2015 07:35
        +1

        Но else не может быть «открывающей скобкой»! Оно и по смыслу, и по синтаксису является инфиксной операцией!


    1. domix32
      02.11.2015 23:43
      +1

      Ребята из idSoftware c вами также не согласятся.


      1. xenohunter
        03.11.2015 14:35
        +1

        Почему это? Вот кусок кода из 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;
        }


        1. domix32
          03.11.2015 17:21

          В упор не вижу два стиля. Везде используется

          if(cond){
              //code
          }
          

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

          Ко всему первый вариант написания присуствует в кодстайле idSoftware. Еще то ли на хабре, то ли на гиктаймс был перевод по их конвенции и коду кажется первой Quake


          1. xenohunter
            03.11.2015 21:16

            Я про else, про то, что перенос скобки на следующую строку присутствует.


            1. VolCh
              08.11.2015 14:45

              ВО многих стайл-гайдах есть и тот, и другой вариант, но варианты использования четко прописаны, например: классы, функции, методы — всегда переносим { на новую строку, а в условиях и циклах всегда оставляем с самим условием или циклом.


    1. Jofr
      03.11.2015 17:55

      А зачем их искать? Раньше я тоже был адептом вынесения фигурной скобки на отдельную строчку, пока вдруг не увидел, сколько визуального места они съедают. Вместо этого можно ввести строгое правило:

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

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

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

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

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


      1. romy4
        03.11.2015 18:37

        Хорошо, опишу, что я подразумеваю. Номер ноль пункт. Открывающие скобки есть не всегда и с этим надо смириться. Особенно, читая чужой код. В коде ниже я взял такой вариант (из комментариев) и в дальнейшем отформатировал для читабельности (сравните?). В первом пункте, фигурный скобки переносятся на новую строку, чтобы вертикально бегая глазами можно было легко найти её открытие без наведения на закрывающую скобку курсора (это экономит время). Второй пункт, следует из правила, что логически не атомарные блоки должны отделяться между собой одной пустой строкой для упрощения чтения. Любое условие 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);
        


        1. khim
          03.11.2015 18:50
          +1

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

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


          1. romy4
            03.11.2015 19:05

            Это не вопрос объективного удобства, это вопрос привычек.


            1. Jofr
              03.11.2015 20:21

              del – запутался в репостах, мысль уже раскрыта


          1. romy4
            03.11.2015 19:10

            Это не вопрос объективного удобства, это вопрос привычек.

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

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

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

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

            Отлично поддерживаются в «плюсах», согласен, их там и надо применять вместо /* */. А где-то не поддерживаются (php).


        1. Jofr
          03.11.2015 20:39
          +1

          В качестве еще одного аргумента, BNF-запись вида (синтаксис упрощен)

          statement =:: IF condition THEN statements END
          

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

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

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

          Вы поймите, я не хочу ломать ваш гайд и ваши привычки =) Просто наше мнение тоже имеет право на существование, вкупе со всеми приведенными выше и ниже обоснованиями.


          1. romy4
            03.11.2015 22:08

            Спасибо большое за объяснение. И вашим style-guide пользуется большинство, как показывает практика.


          1. Mingun
            06.11.2015 19:49
            -1

            так как полностью исключает проблему блуждающего ELSE.

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


            1. Jofr
              06.11.2015 20:08
              +1

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

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

              Конечно же, нет никаких сложностей запомнить «правило ближайшего IF», просто в данном случае это еще один, пусть и небольшой, но довод в пользу строгого правила «ставить скобки всегда».


              1. Jofr
                06.11.2015 20:18

                UPD: У С++ грамматика LALR(1), просто чтобы быть точным.


                1. khim
                  07.11.2015 00:49

                  К сожалению у C++ LALR(?) грамматика, как всем известно.


                  1. Jofr
                    07.11.2015 12:20

                    Ух, значит, не всем. Любопытнейше, спасибо за уточнение.


              1. Mingun
                06.11.2015 21:30
                -1

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

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

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


                1. Jofr
                  06.11.2015 22:35

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

                  Очень хорошо ситуация разобрана здесь:
                  https://en.wikipedia.org/wiki/Dangling_else


            1. dtestyk
              06.11.2015 20:11

              он принадлежит последнему IF-у?
              А что делать, когда нужно, чтобы он относился, например, к предпоследнему?


              1. Mrrl
                06.11.2015 20:54
                +1

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


  1. mitaichik
    02.11.2015 21:51
    +4

    Хорошая работа, но, имхо, любой подобный пост будет слишком поверхостным. Именно поэтому в «Совершенном коде» около 900 страниц.

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

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

    Я в таких случая рекомендую книгу Эрика Эванса DDD, но народ читать не любит, увы.


    1. BloodJohn
      03.11.2015 12:01

      Я копну еще глубже (из собственной практики):

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

      Если начать натаскивать на нейминг и комментирование — остальное решается само собою.


      1. DrLivesey
        03.11.2015 13:42

        Я на себе чувствую проблемы с придумыванием правильного имени для функции/переменной/класса. Иногда получается что имена разные, но похожи друг на друга. Как можно «натаскаться» на правилное именование?


        1. Lure_of_Chaos
          03.11.2015 13:59

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


          1. Keyten
            04.11.2015 02:45

            особенно это относится к введению общего предка совершенно различных классов ради экономии кода

            Вот это как раз про композицию.


      1. Avers
        03.11.2015 14:09
        +1

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


    1. VolCh
      08.11.2015 15:07
      +1

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


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


  1. Mercury13
    02.11.2015 23:22
    +5

    Лучше 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.



  1. BloodJohn
    03.11.2015 11:44

    Лучше тогда вот так.

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

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

    P.S. За египетские скобочки — поубивавбы.


  1. Lure_of_Chaos
    03.11.2015 13:35
    +2

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

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

    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; вообще следует применять физическое насилие, поскольку здесь имеется доморощенная константа нижайшей точности при имеющейся библиотечной константы, и уже молчу про написание константы в нижнем регистре.


    1. Argutator
      03.11.2015 18:44

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

      createField(data, withButton)

      чем

      createField(data, true)

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

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

      ИМХО, автор просто привел неудачный пример


    1. VolCh
      08.11.2015 15:30

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

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

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


      1. Lure_of_Chaos
        08.11.2015 16:31

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

        Вы правильно упомянули проблемы «магических чисел», но я писал о той проблеме, что эти числа заданы жестко и смысл их выбора непонятен. Тот же maxWindowSize/2 прояснил бы логику автора кода. Особенно страшно это выглядит в жуткой негибкости кода, налицо проблемы при работе с разными разрешениями и пр.


  1. romy4
    03.11.2015 15:59

    Больше идеологический вопрос: вы перекладываете обработку неправильных параметров на функцию или оставляете это на совести передающего? Если на функцию, то какое поведение должна избирать функция: проигнорировать или выдать ошибку? А если и на пользователя, и на функцию, то как вы относитесь к постоянному дублированию проверок?


    1. isxaker
      03.11.2015 17:18

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


      1. romy4
        03.11.2015 18:38

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


        1. isxaker
          03.11.2015 20:00

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


    1. VolCh
      08.11.2015 15:49

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

      if(obj.isValidParamForDoSmth(param)) {
          obj.doSmth(param);
      }
      

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


  1. isxaker
    03.11.2015 17:22

    на счет осмысленных названий для переменных

    int delay = 600;
    //vs
    int delayInSecond = 600;
    


  1. Asen
    03.11.2015 20:42

    По сути, статья кратко излагает первые 7(кажется, так) глав КодКомплита,
    а в этих главах примерно 80% того, что нужно знать для создания хорошего кода.
    Поэтому польза, конечно же есть. «Краткость сестра таланта»(с)


  1. isxaker
    03.11.2015 21:12

    //так
    if (a == b)
    {
       //code 1
    } else {
       //code 2
    }
    //или так
    if (a != b)
    {
       //code 2
    } else {
       //code 1
    }
    


    1. VolCh
      08.11.2015 15:52

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


  1. dtestyk
    04.11.2015 01:21
    +1

    Немного наглядных рассуждений:

    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


  1. michael_vostrikov
    04.11.2015 11:07

    (не ради разжигания холивара, просто личные наблюдения)

    Заметил, что даже в проектах, где принят стиль с египетскими скобками, иногда для улучшения читабельности все равно ставят после 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;
    }
    


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


    1. VolCh
      08.11.2015 15:57

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


      Посмотрел пример с doctrine (прямо сейчас пишу для неё) — там пустыми строками отделено не тело цикла от оператора цикла, а логические блоки друг от друга.


  1. zip_zero
    06.11.2015 00:06

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


    1. VolCh
      08.11.2015 16:00

      Но если он себя (или начльство его) заставит следовать стайл-гайдам, то его код будет более красивым.


      1. khim
        08.11.2015 16:54

        С точки зрения создателя стайл-гайда — да, с точки зрения того, кого заставили следовать стайл-гайдам — скорее всего нет.

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

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