>
Главная > Статьи

Главный принцип хорошего кода

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

Твой план действий:

15 минут: Прочтение статьи,
10 минут: Осмысление методики,
30 минут: Ощутимые результаты в твоем проекте.

Итак,

Зачем?

Что можно сказать о плохом коде?

  • ничего не понятно
  • непонятные переменные
  • огромные методы
  • неочевидные алгоритмы
  • хаки, нарушения инкапсуляции
  • высокая сложность и запутанность
  • изменения приводят к сюрпризам
  • ничего не понятно

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

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

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

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

Важно!

Все примеры в статье — синтетические. Совершенно нет никакого смысла пытаться мысленно исправлять код, читая примеры. Здесь речь не об этом.

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

function locator($device) {
  // Can we locate the device?
  if ($device->token != "" && $device->expire <= $now) {
    return false;
  }

  $modelNotifier = new ModelNotifier($device);
  return $modelNotifier->go();
}

Вроде кажется, что это более-менее чистый код. Но это только так кажется.

  • Каков тип $device?
  • Какой тип у $device->expire и $now? Это точно число или там происходит неявное сравнение дат?
  • Что возвращает $modelNotifier->go()?

Это код из реального проекта. Для того чтобы понять, что на самом деле делает этот код, тебе нужно будет поискать в проекте все упоминания $device->expire и найти место, где оно инициализируется. Тебе нужно будет открыть ModelNotifier->go() и по коду выяснить, что он возвращает. Более того, если ты впервые открываешь этот проект, то ты не знаешь что такое token и что означает в нем пустая строка. На все это ты в лучшем случае потеряешь время, а в худшем случае сломаешь рабочую систему.

Мало того, многоэтажные конструкции условий и вызовы методов без переменной создают когнитивную нагрузку: тебе нужно в уме откомпилировать несколько кусочков кода типа token!="", а потом в уме же собрать цепочку условий из результатов. Это происходит автоматом, но нагружает мозг, даже если ты этого не чувствуешь.

Следовательно, нам нужен простой способ писать легкочитаемый код. Желательно — механический. Некая методика, которую может применить и профессиональный и юный программист; пригодная для утра и для вечера, для существующего и для нового кода. Методика, дающая гарантированный результат.

Такая методика существует.

Как?

Все, что можно отделить и назвать — должно быть отделено и названо.

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

Посмотри еще раз:

function locator($device) {
  // Can we locate the device?
  if ($device->token != "" && $device->expire <= $now) {
    return false;
  }

  $modelNotifier = new ModelNotifier($device);
  return $modelNotifier->go();
}

В начале этого метода идет отдельный кусочек кода, который решает отдельную задачу, дает ответ на отдельный вопрос: “можно ли найти девайс?”

Этот код можно отделить и назвать:

function ModelDevice::isLocatable() {
  return ($this->token != "" && $this->expire <= $now);
}

…

function locateAndNotifyDevice(ModelDevice $device) {
  if (!$device->isLocatable()) {
    return false;
  }

  $modelNotifier = new ModelNotifier($device);
  return $modelNotifier->go();
}

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

Идем дальше. Давай посмотрим внутрь isLocatable(). В начале видим тоже самое: в этом методе есть два алгоритма, которые отвечают на совершенно разные вопросы:

  1. Есть ли у нас токен?
  2. Не истек ли срок действия?

Обрати внимание: эти вопросы вообще не касаются того, возможно ли найти девайс.

Выделяем методы:

function isDeviceExpired() {
  return ($this->expireAtUnixtime >= $now);
}

function isOurTokenValid() {
  return ($this->token != "");
}

function isLocatable() {
  return ($this->isOurTokenValid() && !$this->isDeviceExpired());
}

Этот код уже похож на нормальную английскую речь. Его когнитивная нагрузка очень низкая: читая метод isLocatable() ты видишь, какое он принимает решение и фокусируешь свое внимание только на высоком уровне. Читая isDeviceExpired(), ты понимаешь, как это решение принимается и твое внимание сосредоточено на конкретных данных.

Этот код уже не требует анализа — его достаточно лишь только прочитать.

Заодно, кстати, ты узнал что означает, когда token пустой. Обрати внимание, каким элегантным и надежным способом это знание закреплено в проекте: названием метода.

Ах да, заметь: этому коду уже не нужны комментарии. Instant win!

Почему?

Это только кажется, что это все слишком просто и тривиально. Отнюдь! Этот принцип гораздо глубже, чем тебе показалось.

Your stupidity
http://www.flickr.com/photos/photonquantique/4139568026/

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

Во-вторых, мозг по-разному осмысливает стратегические (“что делать?”) и тактические (“как делать?”) задачи. Когда ты пишешь метод isLocatable(), ты сфокусирован лишь на одной маленькой и четкой задаче: как определить, может ли девайс быть обнаружен или нет. Это — тактическое мышление, предельно внимательное к деталям и весьма техничное. А вот когда ты пишешь “главный” метод locateDevice(), ты не думаешь над мелкими вопросами; ты создаешь, собственно, прикладную логику приложения. Это — стратегическое мышление, оно творческое и мыслит абстракциями.

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

* 300ms — средняя скорость переключения контекста мышления по разным оценкам и исследованиям.

Примеры

Инициализация конфига:

function getConfig() {
  if ($this->hasOption('configs')) {
    $configs = $this->getOption('configs');
  } else if ($this->hasOption('config')) {
    $configs = [ $this->getOption('config') ];
  }

  if (isset($configs)) {
    $root = $this->getOption('root', null);
    if (!$root) {
      if (isset($_SERVER['PWD'])) {
        $root = $_SERVER['PWD'];
      } else if (isset($_SERVER['DOCUMENT_ROOT'])) {
        $root = $_SERVER['DOCUMENT_ROOT'];
      }
    }

    $configs->root = $root;
  }

  return $configs;
}

Два алгоритма: один читает конфигурацию в одном из двух вариантов (множественный и единичный), второй находит корневой каталог проекта. Разделяем и даем имена:

function grabConfiguration() {
  if ($this->hasOption('configs')) {
    return $this->getOption('configs');
  } else if ($this->hasOption('config')) {
    return [ $this->getOption('config') ];
  }
}

function figureOutRootFolder() {
  $root = $this->getOption('root', null);
  if (!$root) {
    $root = $_SERVER['PWD'];
  }
  if (!$root) {
    $root = $_SERVER['DOCUMENT_ROOT'];
  }
  return $root;
}

function getConfigs() {
  $configs = $this->grabConfiguration();
  if (!is_null($configs)) {
    $configs->rootFolder = $this->figureOutRootFolder();
  }
  return $configs;
}

Код поиска картинки в мобильном приложении на iOS:

if ([imageEntry objectForKey:@"fullscreenUrl"] == nil) {
  NSString *kind = [imageEntry objectForKey:@"kind"];
  if ([kind isEqualToString:@"pdf"]) {
    UIImage *i = [UIImage imageNamed:@"galleryPdfIcon"];
    [self.uiImagesCache setObject:i forKey:@(photoIndex)];
    *photoSize = NIPhotoScrollViewPhotoSizeOriginal;
    return i;

  } else if ([kind isEqualToString:@"video"]) {
    UIImage *i = [UIImage imageNamed:@"galleryVideoIcon"];
    [self.uiImagesCache setObject:i forKey:@(photoIndex)];
    *photoSize = NIPhotoScrollViewPhotoSizeOriginal;
    return i;

  } else {
    UIImage *i = [UIImage imageNamed:@"galleryBrokenImage"];
    [self.uiImagesCache setObject:i forKey:@(photoIndex)];
    *photoSize = NIPhotoScrollViewPhotoSizeOriginal;
    return i;
  }
}

Если картинки нет, то нужно взять общую картинку. Результат положить в кэш. Разделяем:

- (UIImage *) genericImageForKind:(NSString *) imageKind {
  UIImage *candidate = nil;

  if ([kind isEqualToString:@"pdf"]) {
    candidate = [UIImage imageNamed:@"galleryPdfIcon"];
  } else if ([kind isEqualToString:@"video"]) {
    candidate = [UIImage imageNamed:@"galleryVideoIcon"];
  } else {
    candidate = [UIImage imageNamed:@"galleryBrokenImage"];
  }

  return candidate;
}

…

if ([imageEntry objectForKey:@"fullscreenUrl"]==nil) {
  NSString *kind = [imageEntry objectForKey:@"kind"];
  UIImage *genericImage = [self genericImageForKind:kind];
  if (genericImage) {
    [self.uiImagesCache setObject:genericImage forKey:@(photoIndex)];
    *photoSize = NIPhotoScrollViewPhotoSizeOriginal;
  }

  return nil;
}

Распиливаем данные из SQL в память. Я этот код писал сам год назад, а сейчас, когда искал примеры для статьи, долго пытался понять, что вся эта каша делает.

function(err, dResults) {
  if (err) {
    console.log(err);
    callback(err);
    return;
  }

  var i=0, lastDownloaded = null;
  var unixtimes = Object.keys(dResults);
  for(i=0;i<unixtimes.length;i++) {
    if (dResults[unixtimes[i]].isDownloaded) {
      lastDownloaded = unixtimes[i];
    }
  }

  for(i=0;i<unixtimes.length;i++) {
    if (unixtimes[i]>lastDownloaded) {
      delete dResults[unixtimes[i]];
    }
  }

  self.getOpenPrice(symbol, function(err, openPrice) {
    callback(err, dResults, openPrice);
  });
}

Разделяем:

function getLastDownloadedUnixtime(entries) {
  var lastDownloadedUnixtime = null;

  var unixtimes = Object.keys(entries);
  unixtimes.forEach(function(unixtime) {
    if (entries[unixtime].isDownloaded) {
      lastDownloadedUnixtime = unixtime;
    }
  });

  return lastDownloadedUnixtime;
}

function cleanupEntriesNewerThanLastDownloadedUnixtime(entries, lastDownloadedUnixtime) {
  var unixtimes = Object.keys(entries);
  for(var i=0;i<unixtimes.length;i++) {
    if (unixtimes[i]>lastDownloadedUnixtime) {
      delete entries[unixtimes[i]];
    }
  }
}

…

function(err, dResults) {
  if (err) {
    console.log(err);
    callback(err);
    return;
  }

  var lastDownloadedUnixtime = getLastDownloadedUnixtime(dResults);
  cleanupEntriesNewerThanLastDownloadedUnixtime(dResults, lastDownloadedUnixtime);

  self.getOpenPrice(symbol, function(err, openPrice) {
    callback(err, dResults, openPrice);
  });
}

Собираем XML для беседы с Amazon S3. Поскольку мы точно знаем, какие данные мы туда кладем, то XML создается вручную:

strcat(xml, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
strcat(xml, "<Delete>\n");
strcat(xml, "<Quiet>true</Quiet>\n");

for (uint32_t i=0;i<batchCount;i++) {
  char *escapedPath=Deleter::xmlEscape(batch[i]);
  strcat(xml, "\t<Object><Key>");
  strcat(xml, escapedPath);
  free(escapedPath);
  strcat(xml, "</Key></Object>\n");
  LOG(LOG_DBG, "[Delete] %s", batch[i]);
}

strcat(xml, "</Delete>\n");

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

void addPathToDeleteXml(char *xml, char *path) {
  char *escapedPath=Deleter::xmlEscape(path);
  strcat(xml, "\t<Object><Key>");
  strcat(xml, escapedPath);
  strcat(xml, "</Key></Object>\n");
  free(escapedPath);
}

void addBatchOfPathsToXml(char *xml, char **batch, uint32_t batchCount) {
  for (uint32_t i=0;i<batchCount;i++) {
    addPathToDeleteXml(xml, batch[i]);
    LOG(LOG_DBG, "[Delete] %s", batch[i]);
  }
}

…

strcat(xml, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
strcat(xml, "<Delete>\n");
strcat(xml, "<Quiet>true</Quiet>\n");

addBatchOfPathsToXml(xml, batch, batchCount);

strcat(xml, "</Delete>\n");

Альтернативное мнение

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

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

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

Как внедрить

Таким рефакторингом можно заниматься в любом коде — он от этого будет становиться гарантированно лучше. Этим можно заниматься в любое время, даже когда голова плохо соображает или устал: ты не меняешь код, ты разделяешь его, это обычно безопасно.

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

Не стоит бояться длинных имен. Напротив, байты экономить не надо, подробные имена здорово облегчают чтение кода, а набирать их в современном редакторе все равно помогает autocomplete.

Комментарии? Замечания? Прошу на Хабрахабре, в ЖЖ или почтой.

Последние статьи
Главный принцип хорошего кода
Об эффективных багрепортах
Достоверный бизнес-план для чайников
Теги
IT8
бизнес7
перевод3
программирование2
фотография3
Об авторе
Егор Егоров

Управляющий партнер и технический директор группы IT-компаний. Программист с более чем 20 годами опыта в разработке юниксовых высоконагруженных приложений и специализированного финансового софта.