О плохом и хорошем коде для чайников

Предисловие

Для того, чтобы написать хороший код завтра,

программист должен писать хороший код сегодня,

читая хороший код, написанный вчера.

Жил-был программист. Работал в большой команде, писал небольшие модули для общего продукта, и в глубине души думал, что код его идеален. Ну, может быть, и не совсем, но наш герой стремился к тому, чтобы эта его уверенность не слишком расходилась с реальностью. Реальность же, как всегда, была жестока: коллеги-программисты за глаза частенько звали его редиской (а иногда и чем похуже), код в один прекрасный день начисто отказался работать, а старшие товарищи в Code Review только и делали, что писали сплошные TODO. Казалось бы, в чём проблема? Ведь всё же работает!

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

Как же обеспечить простоту и очевидность кода?

Стандарты написания кода и с чем их едят

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

Именование объектов

- Я человек простой, вижу переменную – называю её x!

Неизвестный программист-математик

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

1. Передавая намерения программиста, имя объекта должно отвечать на три вопроса: почему этот объект существует, что он делает и как используется. Если имя требует дополнительных пояснений, значит, оно не передаёт этих намерений. Сравните два варианта: reference и ContractRecordForSign, какой из них более информативен?
2. Разные имена должны обозначать разные понятия, и наоборот. Недостаточно назвать переменные User1 и User2, лучше, если это будут SimpleUser и AdminUser. Также при присвоении различных имён нужно стремиться к тому, чтобы читатель кода мог понять, какой смысл заложен в этих различиях.
3. Ясность имени имеет более высокий приоритет, чем его краткость. Не нужно искусственно ограничивать длину имён объектов, потому что главная задача имени – максимально полное и понятное описание сути объекта (в разумных пределах, само собой). Однобуквенные имена лучше использовать только для переменных цикла.
4. При сокращении длинного имени нужно придерживаться одного и того же варианта. Нельзя в одном сценарии сократить ReferenceRecord как RefRec, а в другом – как Ref.
5. Имя должно легко читаться и произноситься. Для этого слова в именах разделяют заглавными буквами или спецсимволами. Обычно применяются стили camelCase, UpperCamelCase, SCREAMING_SNAKE_CASE. При этом предыдущее правило продолжает действовать: выбрав для именования какого-либо вида объектов один стиль, необходимо придерживаться его в дальнейшем.
6. Необходимо избегать малозаметных различий в именах во избежание путаницы. Здесь следует отдельно отметить, что ISBL не чувствителен к регистру символов, поэтому переменные sqlQuery и SQLQuery будут считаться одной и той же переменной.
7. Очевидно, но необходимо избегать орфографических ошибок и опечаток в именах, поскольку это затрудняет поиск таких имён в коде. Отсюда же можно сделать вывод, что не следует использовать в именах слова, при написании которых часто возникают ошибки.
8. Использование транслита или русских букв не запрещено, но считается дурным тоном.

Небольшой пример, просто для понимания, почему настолько важно правильное именование объектов. Когда автор этой статьи учился на 1 курсе университета, у него была дурная привычка давать переменным в коде однобуквенные имена. В течение одного только первого семестра мы написали больше двух десятков коротких программ, и написали их настолько хорошо, что спустя некоторое время возникла необходимость вернуться к ним для наставления младших товарищей. Архив с задачами нашёлся быстро, файлы открылись ещё быстрее, а дальше наступила прострация. Что делает переменная a? Для чего введена константа b? Что за таинственная s выводится в качестве результата? Несмотря на то, что это был наш же код, вспомнить суть его работы нам удалось не сразу. Конечно, этот пример слегка утрирован. На самом деле там было не вычисление площади, а расчёт основных характеристик треугольника по заданным координатам его вершин, но согласитесь, что понять всё было бы намного проще, будь вместо a – width, b – fixedLength, а вместо s – area.

Оформление

- А что, вы и код за меня оформлять будете?

- Ага!

(диалог программиста-новичка и Visual Studio)

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

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

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

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

  try
    Card = References.ReferenceFactory(CompCode).GetObjectByID(CardID)
  except
    Raise(CreateException('ReferenceRecordNotFound'; LoadStringFmt('DIRA7CF9B83_A3B6_4F79_8CDE_589C4F2E6644'; 'COMMON'; ArrayOf(CompCode; CardID)); ecException))
  endexcept

  try
    Card = References.ReferenceFactory(CompCode).GetObjectByID(CardID)
  except
    Raise(CreateException('ReferenceRecordNotFound';
                          LoadStringFmt('DIRA7CF9B83_A3B6_4F79_8CDE_589C4F2E6644'; 'COMMON'; ArrayOf(CompCode; CardID));
                          ecException))
  endexcept

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

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

  pickState = SQL('Select PriznValues From MBRecvAn Where FldName = "RefStatus"')
  indexState = SubString(pickState; ';'; 5)
  if indexState <<>> ''
    State = SubString(pickState; ';'; indexState)
  else
    State = ''
  endif

pickState = SQL('Select PriznValues From MBRecvAn Where FldName = "RefStatus"')
indexState = SubString(pickState; ';'; 5)
if indexState <<>> ''
State = SubString(pickState; ';'; indexState)
else
State = ''
endif

На первый взгляд может показаться, что разница не так уж и велика, но когда длина сценария достигает хотя бы 100-150 строк (а хорошо, вдумчиво написанные сценарии на ISBL могут иметь и 500, и все 1000), она начинает критически влиять на восприятие кода. Пока код работает, как надо, это может никого не задевать, но при любом сбое даже его автору может потребоваться большое количество времени, чтобы просто найти проблемное условие.

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

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

Комментарии

- И вообще, зачем это всё?

Риторический вопрос программиста, занятого чтением чужого кода

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

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

Несмотря на всё это, существуют и полезные комментарии, такие как пояснения к формату даты-времени, ошибки и недокументированные возможности используемой платформы, документирование неочевидных подходов, резюме кода и т.д. Отдельно выделяют комментарии TODO, используемые как способ оставления заметок на будущее.

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

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

Вместо заключения

По статистике, отношение времени чтения старого кода ко времени написания нового превышает 10 к 1. То есть в среднем 90% времени программист тратит на просмотр чужого кода (как правило, для поиска возможностей его исправления/модификации или просто для того, чтобы сделать нечто аналогичное), и при этом зачастую даже не может сделать выбор между плохим и хорошим кодом, потому что хорошего кода просто нет. И исправить это (написать хороший код вместо плохого) могут только сами программисты.

Грустно, когда чужой код выглядит вот так:

  Params = Object.WorkflowParams
  i = 0
  if (Params.ValueByName('RepealedGroups').Count > 0)
//     while i <= Params.ValueByName('RepealedGroups').Count-1
       //Params.ValueByName('RepealedGroups').Values(i).FullName == 'Purchasing' //or
//         Params.ValueByName('RepealedGroups').Values(i).FullName <<>> 'MainData' //or
         //Params.ValueByName('RepealedGroups').Values(i).FullName == 'SAPAdmin'
         
         //  добавление от 23.09.2016-------------------------------------------
         if Params.FindItem("Rez_1").value == 1
            Res = '1'
         endif
         if Params.FindItem("Rez_1").value == 2
            Res = '2'
         endif
//         if Params.FindItem("Rez_1").value == 1 or Params.FindItem("Rez_1").value == 2
//            Res = '3'
//            Params.ValueByName('NextApproverNum').Value = Params.ValueByName('NextApproverNum').Value + 1
//         endif    
        // конец изменения от 23.09.2016 ---------------------------------------
         if Params.FindItem("Rez_1").value == 4
            Res = '4'
         endif
         if Params.FindItem("Rez_1").value == 5
            Res = '5'
         endif
//      else
//        if Params.FindItem("Rez_1").value == 1
//          //Params.ValueByName('NextApproverNum').Value = Params.ValueByName('NextApproverNum').Value + 1
//           Res = '1'
//        endif
//        if Params.FindItem("Rez_1").value == 2
//          //Params.ValueByName('NextApproverNum').Value = Params.ValueByName('NextApproverNum').Value + 1
//           Res = '2'
//           Params.ValueByName('Repealed').Value = 1
//        endif
//        if Params.FindItem("Rez_1").value == 4
//           Res = '4'
//        endif
//        if Params.FindItem("Rez_1").value == 5
//           Res = '5'
//           endif

Или вот так:

// Формирование наименования
Точн(3)
if Assigned(Object.Initiator) or Assigned(Object.Admin)
Name = ''
if Assigned(Object.Requisites('PersonName').DisplayText)
  Name = Object.Requisites('PersonName').DisplayText
endif

Object.Наименование = Object.SysReq_Code & ' - ' & Name
DataSet3 = Object.DetailDataSet(3)  
DataSet4 = Object.DetailDataSet(4)

DataSet5 = Object.DetailDataSet(5)
  i = 0
 foreach rec in DataSet4
    if (DataSet4.sostojanie == "Выполнено") 
      i = i +1
    endif
 endforeach

//DataSet6 = Object.DetailDataSet(6)
//DataSet6.Index = 'StepNumber;ASC' 
//DataSet6.Indexed  = True
///////////////////////////////////////////////////////////////
/////////////       даты      /////////////////////////////////
///////////////////////////////////////////////////////////////   
   
counter = 0                                //счетчик
count = 0                                  //счетчик
Arr =ArrayOf()                             //массив дат по таблице
Arr2 = ArrayOf()                           //массив уникальных дат (для проверки количества дат)
Arr3 = ArrayOf()                           //массив для подсчета
//DataSet4 = Object.DetailDataSet(4) 
punkt = '' 

А вот так – почти не грустно:

// Значения по умолчанию
if not assigned(Object.RefStatus)
  ref_status = CreateReference('Statuses')
  ref_status.open
  ref_status.locate('Наименование'; 'Черновик')
  Object.Requisites('RefStatus').Value = ref_status.SYSREQ_CODE
  ref_status.close
  ref_status = nil
endif
if not assigned(Object.date_create)
  Object.date_create = ToDay()
endif
if not assigned(Object.ForContract)
  Object.ForContract= 'Да'
endif
if not assigned(Object.ДаНет)
  Object.ДаНет = 'Нет'
endif
if not assigned(Object.ДаНет2)
  Object.ДаНет2 = 'Нет'
endif
if not assigned(Object.ДаНет3)
  Object.ДаНет3 = 'Нет'
endif

Каждый раз, когда мы пишем плохой код, где-то в мире начинает грустить наш коллега. Давайте не будем редисками, потому что писать хороший код – просто. Попробуйте сами!

P.S. Небольшой интерактив для особо пытливых: а что не так в последних трёх примерах и почему? 

Продолжение О плохом и хорошем коде для чайников. Часть 2