Василий Половнёв

Разработчик, зануда, сооснователь Kursk Meetup.

Специализация — Рельсы, фронтенд, автоматизация тестирования и разработки.

Ушпешный блог Почта Твиттер Гитхаб СЯУшки

Дезодоранты и полезные комментарии

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

Смотрите:

def archive
  # unlock and destroy everything in project
  discussions.each(&:unlock!)
  todos.each(&:unlock!)
  discussions.destroy_all
  todos.destroy_all

  # assign read-only role to users
  users.each do |user|
    user.update_attribute(role: "client")
  end

  self.status = 38 # set status to archived
end

Чтобы сделать код понятнее, вытащите константу и несколько методов с вразумительными именами:

def archive
  clean_up_before_archiving
  reset_user_roles_to_read_only

  self.status = ARCHIVED
end


def clean_up_before_archiving
  discussions.each(&:unlock!)
  todos.each(&:unlock!)

  discussions.destroy_all
  todos.destroy_all
end

def reset_user_roles_to_read_only
  users.each(&:reset_role)
end

БА ДУМ ТСС! Отсюда правило:

Если код не понять без комментария, рефактори

Полезные комментарии

Комментарии, объясняющие почему код написан именно так, помогают узнать предысторию и изначальные намерения разработчика. Чаще всего это многострочные, развернутые заметки с ссылками на дополнительную информацию. Например, когда мы манки-патчим библиотеку:

# Monkey patching Paperclip to disable built-in spoofing protection
# which gives many false-positive errors and prevents uploading of
# .xlsx, .caf, .msg files.
#
# For more details see:
#
# * http://robots.thoughtbot.com/prevent-spoofing-with-paperclip
# * https://github.com/thoughtbot/paperclip/issues/1456
# * https://github.com/thoughtbot/paperclip/issues/1530

module Paperclip
  class MediaTypeSpoofDetector
    def spoofed?
      false
    end
  end
end

Или объясняем, для чего здесь этот хак:

.is-visible & {
  // Adding transparent outline fixes jagged edges in Firefox.
  //
  // See: http://stackoverflow.com/a/9333891
  outline: 1px solid rgba(255, 255, 255, 0);
}

Наверняка бывают и другие типы полезных комментариев. Если вы их встречали, покажите-расскажите: vasily@polovnyov.ru.

###

Чтобы этот пост активнее шарили в Фейсбуке, прикладываю картинку-правило, заряженную на рефакторинг и быстрые тесты:

По хардкору: instance_double, verify_partial_doubles

В комментариях к посту о дублерах, стабах и моках Макс Прокопьев упомянул instance_double, секретную технику старших разработчиков. О ней и поговорим. Коротко, по делу и с примерами.

Ситуация

Возьмем пример с Notifications:

class Notifications
  def as_json
    # Загружаем и отдаем последние новости
    # и уведомления с серверов Юрия Лозы по ХТТП
  end
end

class NotificationsController
  def index
    # Оборачиваем уведомления в ключ 'data'
    render json: { data: notifications.as_json }
  end

  def notifications
    Notifications.new
  end
end

Чтобы не обращаться к серверам Лозы в тестах, используем дублера:

describe "NotificationsController#index" do
  let(:notifications) do
    double(:datasource, as_json: { notifications: [] })
  end

  before do
    allow(Notifications)
      .to receive(:new)
      .and_return(notifications)
  end

  it "wraps notifications in 'data' key" do
    get :index, format: :json

    expect(json_response["data"].keys)
      .to have_key "notifications"
  end
end

Что произойдет, если мы переименуем Notifications#as_json в Notifications#to_json? Ничего. Мы останемся наедине с зеленым тестом, проверяющим бесполезного дублера.

instance_double

Чтобы не попадать в такую дебильную ситуацию, используйте instance_double:

describe 'NotificationsController#index' do
  let(:notifications) do
    instance_double(Datasource, as_json: { notifications: [] })
  end

  # ...
end

Такой дублер проверит свой интерфейс. Если метода нет или у него другие аргументы — красный тест.

Чтобы так же проверять моки и стабы, убедитесь, что verify_partial_doubles включен в spec_helper.rb.


Внимательный читатель со звездочкой наверняка заметил, что одного instance_double мало. Все верно. В RSpec есть похожие дублеры для классов, модулей и объектов: object_double и class_double.

По хардкору: дублеры, моки, стабы

Сегодня о тестах. Пост для тех, кто знаком с RSpec, но не понимает, что такое «мокать» и «застабить». Коротко, по делу и с примерами.

Дублер (test double)

Объект-каскадер, подменяющий реальный объект системы во время тестов:

describe NotificationsController do
  # NotificationsController загружает последние уведомления
  # со стороннего сервиса по HTTP
  # с помощью NotificationsDatasource.
  let(:datasource) do
    double(:datasource, as_json: { notifications: [] })
  end

  before do
    # Подменяем реальный NotificationsDatasource дублером,
    # чтобы не зависеть от внешнего сервиса в тестах:
    allow(NotificationsDatasource)
      .to receive(:new)
      .and_return(datasource)
  end

  describe "#index" do
    it "wraps notifications in 'data' key" do
      get :index, format: :json

      expect(json_response["data"].keys)
        .to have_key "notifications"
    end
  end
end

Стаб (stub)

Заглушка для метода или объекта, возвращающая заданное значение:

context "when attachment file is too large to email" do
  let(:max_file_size) { Attachment::MAX_FILE_SIZE }

  before do
    allow(attachment)
      .to receive(:file_size)
      .and_return(max_file_size + 1)
  end

  it "raises 'file is too large' error" do
    # ...
  end
end

Внимательный читатель со звездочкой уже заметил, что и в предыдущем примере с NotificationsController был стаб. Все верно: стаб — это дублер с зашитыми ответами.

Мок (mock)

Стаб с ожиданиями, которые RSpec проверит в конце теста:

context "when cloning process cannot be performed" do
  before do
    allow(doctor).to receive(:clone) { raise "can't clone" } # стаб
  end

  it "notifies airbrake" do
    expect(Airbrake).to receive(:notify) # мок
    # Rspec застабит `Airbrake.notify`
    # и в конце этого `it do...end` блока
    # проверит, был ли он вызван.
    # Если вызова не было — ошибка и красный тест.

    clone_poor_dolly
  end
end

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

# мок
it "notifies airbrake" do
  expect(Airbrake).to receive(:notify) # проверка + настройка

  clone_poor_dolly # испытание
end

# стаб + проверка
it "notifies airbrake" do
  allow(Airbrake).to receive(:notify) # настройка

  clone_poor_dolly # испытание

  expect(Airbrake).to have_received(:notify) # проверка
end

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

Стоп-комментарии

Без стоп-комментариев код становится лаконичнее и информативнее. Этот пост о комментариях, которым не место в коде.

Закомментированный код отвлекает от чтения и исследования, увеличивает риск пропустить важное. Как пятиминутный рекламный блок, начавшийся на словах Вито Корлеоне: «…ты никогда не искал моей дружбы и ты боялся быть у меня в долгу».

// var fibonacci = function(n) {
//  if (n < 2){
//    return 1;
//  } else {
//    return fibonacci(n-2) + fibonacci(n-1);
//  }
// }

finallyMakeSomethingUseful()

Не предлагайте дружбу. Удаляйте без сожалений. Если что — возьмете из истории Гита.


Капитанские комментарии объясняют очевидное, описывают то, что и так ясно из кода.

# Associations
has_many :posts
has_many :comments
// Default Config
var defaultConfig = { showWordCount: true }

// close connection
connection.close()
// в _header.scss
// ========================
// HEADER
// ========================
#header { }

В них нет пользы и новой информации. Удаляйте без раздумий.


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

// TODO: fix this hack!
// TODO: move it somewhere else
// FIXME: this is not a good idea
// TODO: refactor this file

Удаляйте без сомнений, чините или выносите в ишки-тудушки в Джиру, Бейскемп, Трелло. В очередной спринт по техдолгу доберетесь до них.

P. S. О «вонючих» и полезных комментариях — в следующем посте.


Еще по теме:

Сегодня я узнал

Мы каждый день узнаем новое: как инлайнить SVG в CSS, как использовать ES2015 в Gulpfile, где работает position: sticky. Накапливать такие знания публично — полезно: растет коллективный опыт. Кроме того, так проще заметить застой в профессиональном росте: перестали поступать новые знания → отстой, деградация, работа риэлтором.

Поэтому я создал репозиторий с заметками-зарубками о том, что нового я узнаю:

Заодно посмотрите и подпишитесь на другие коллекции:

Три правила ревью

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

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


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

Поэтому правило: открыл пуллреквест → попил чайку → отревьюил. Только чаепитие не пропускайте: это отличный способ отвлечься и взглянуть на код со стороны.

Первый ревьюер — ты

Если изображать Синтаксический Анализатор Пуллреквестов 3000 и при ревью придираться к скобочкам, кавычкам и переносам, легко пропустить важное. Чтобы избавить себя от тупой работы по проверке синтаксиса и освободить время, настройте линтеры.

Отсюда и правило покруче: если что-то можно проверить автоматически, это должен делать робот, а не человек.

Синтаксис — роботам

Чтобы ничего не забыть при ревью, используйте закрытые списки. Вот список вопросов, по которым я прохожусь при ревью пуллреквеста в проекте на Рельсах:

  • понятно как и какую проблему решает пуллреквест?
  • гифка в пуллреквесте смешная?
  • в коде нет потенциальных багов?
  • от кода не пахнет (DRY, SRP, комментарии)?
  • имена и названия понятны?
  • код легко читается?
  • тесты на месте?
  • тестов достаточно, нет лишних?
  • по изменениям в тестах понятно, что изменилось в коде?
  • миграции и изменения в схеме совпадают?
  • у добавленных в БД полей есть нужные индексы и ограничения?
  • не торчит где-нибудь XSS или SQL инъекция?
  • не налажали с проверками доступа?
  • роуты в порядке?
  • нет жирных моделей, контроллеров и вьюх?
  • колбэки меняют только собственное состояние?
  • нет медленных участков (N+1 запросов, O(n^2) алгоритмов)?
Проверяй по списку

Если ваш список правил и проверок при ревью круче, поделитесь опытом, напишите: vasily@polovnyov.ru.


Еще по теме:

Управление временем

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

Производительность = талант × самодисциплина

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

Если сомневаетесь, нужен ли вам тайм-менеджмент, поработайте неделю с RescueTime. Посмотрите, сколько времени уходит на работу, а сколько улетает впустую.

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

И 5 советов как не облажаться на старте.

  1. Забудьте про мотивацию. Главное — дисциплина.
  2. Выкрутите себе руки. Система должна быть сильнее вас.
  3. Найдите единомышленников. Отчитывайтесь перед друзьями или близкими за прошедшую неделю, делитесь планами на «год» и следующую неделю.
  4. Освободите голову. Записывайте и планируйте все от «разобрать Покет» и «купить 2 лампочки E17 25 ватт» до «server-side render в Фатеже» и «тестовый деплой с Ansible на Digital Ocean».
  5. Возьмите блокнот и ручку. Вычеркнуть сделанную задачу — удовольствие, которого нет в модных таск-трекерах.

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

Подростковое тестирование. Подборка

Подборка предназначена для разработчиков знакомых с Ruby и Rspec, но не до конца понимающих что и как тестировать. Для тех, кто прочитал Rspec Book, но не может написать тест с нуля. Для тех, кто исправляет баг за 5 минут, а потом 2 часа пишет для него тест. Для тех, кто прохавал жизнь с самого низа.

Если вы не знакомы с Ruby/Rspec, а статьи понять хочется, пройдите эти курсы:

Тестовая пирамида

Модульные тесты

Моки и стабы

UI, интеграционные, приемочные тесты

Дополнительно

Хороший доклад

На прошлой неделе я выступил с докладом об and в Ruby и прослушал 6 программистских докладов. Этот пост для меня и программистов, выступающих на митапах, демо и конференциях.


Хороший доклад — это полезный доклад. Если доклад бесполезен, ребята в зале достанут телефоны и полезут в твиттер.

Если вы начнете изучать Clojure, то сможете лучше ознакомиться с Emacs.

Супер. Если купите автомобиль, будет комфортнее добираться до автосервиса и заправки.

Короче, тут у нас Elastic Search, React, Flux и Docker. Все очень круто работает в продакшене пока мы попиваем картофельный смузи.

Ага, спасибо. Ты клевый. Когда фуршет?

Ну, я, короче, не знаю, что вам еще показать. Вот как-то так оно и работает.

Ясно-понятно. Спасибо, что украл всего час времени.

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

Автопрефиксер сам расставит нужные префиксы. Больше никаких миксинов с бордер-радиусами и градиентами.

С CSS инбоксами программисты пишут CSS без стыда и боязни перед профессиональными верстальщиками.

Как бы вы не старались точно оценить задачу, все равно ошибетесь. Лучше переоценить задачу по времени, тогда потери будут минимальны. Недообещать > переобещать.

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

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

Хороший доклад — полезный доклад.

Stylus → PostCSS

Пока серьезные ребята только подумывают о переезде на PostCSS, я взял и перевел сайт (не этот, а другой) со Stylus на PostCSS.

У меня было 400 строк кода, 20 переменных, gulp, собирающий фронтенд, и целое множество правил всех сортов и расцветок, а также медиавыражения, псевдоклассы, миксины и вложенные селекторы. Не то что бы это был необходимый запас для сайта. Но если начал собирать фронтенд, становится трудно остановиться.

PostCSS — парсер, и работает с текстом похожим на CSS. Поэтому в первую очередь я переехал с SASS-подобного синтаксиса (значащие отступы, отсутствие точек с запятой и фигурных скобок) на SCSS-подобный. Stylus поддерживает оба синтаксиса, поэтому это делается до переезда на PostCSS:

// было
.footer
  padding: 1rem
/* стало */
.footer {
  padding: 1rem;
}

Теперь выкидываем Stylus, меняем расширение .styl файлов на .css и добавляем PostCSS. В первую очередь нужен postcss-import для поддержки @import, postcss-simple-vars для переменных и postcss-nested для вложенных селекторов:

postcssImport = require("postcss-import")
postcssVars = require("postcss-simple-vars")
postcssNested = require("postcss-nested")

preprocessors = [
  postcssImport(from: AppConfig.paths.mainStylesheet),
  postcssNested,
  postcssVars
]

gulp.task "stylesheets", ["clean:stylesheets"], ->
  gulp.src(AppConfig.paths.mainStylesheet)
    .pipe(postcss(preprocessors))

Объявление переменных придется поменять ( = на :):

// было
$text-color = rgba(0, 0, 0, .85);
/* стало */
$text-color: rgba(0, 0, 0, .85);

С миксинами сложнее. Есть postcss-mixins:

// было
clearfix() {
}

.footer {
  @include clearfix;
}
/* стало */
@define-mixin clearfix {
}

.footer {
  @mixin clearfix;
}

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

Для работы с цветом есть postcss-color-function:

// было
$footer-color = lighten($text-color, 10%);
$submit-shadow-color = darken($link-color, 20%);
/* стало */
$footer-color: color($(text-color) lightness(10%));
$submit-shadow-color: color($(link-color) blackness(+20%));

Вот и все. Уложился в час.


Следующий свой проект обязательно буду собирать с PostCSS вместо Stylus/LESS/SASS. Все, что нужно мне от препроцессоров, — вложенные селекторы, переменные, миксины и работа с цветом. Все это есть в PostCSS в виде крохотных подключаемых модулей.

В то же время нет безумных штук вроде @for, @if, @at-root, @each из SASS. PostCSS — это ограничения, в которых рождается хороший дизайн и чистый код.

Ну, и он быстрый:

# PostCSS
[21:42:10] Starting 'stylesheets'...
[21:42:10] Finished 'stylesheets' after 206 ms

# Stylus
[21:42:45] Starting 'stylesheets'...
[21:42:45] Finished 'stylesheets' after 325 ms

Дополнительное чтение: