Ruby on Rails - Самые распространенные ошибки новичка. Часть 3 - The ruby way

Всем привет. Продолжаем говорить о стандартных ошибках начинающих Ruby on Rails программистов.

  1. Модели и база данных (11 ошибок всего, из них 4 важных)
  2. Контроллеры (6 ошибок всего, из них 3 важных)
  3. The Ruby Way и качество кода (15 ошибок всего, из них 5 важных)
  4. Общие ошибки без категории (8 ошибок всего, из них 4 важные)

Немного об оформлении. Все ошибки разделены на 4ре категории. Кроме того, есть Особо важные ошибки выделенные красным цветом. Эти ошибки могут критично сказаться на работе проекта и команды.

Третья часть посвящена общепринятым подходам при программировании на ruby, тому, что называется The Ruby Way. 

Итак, новички:

Не до конца понимают, что такое предикаты

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


################################
##       Это плохо!           ##
################################
  
  
  def author?
    if current_user.id ==  course.user.id
      current_user.add_role(‘author’)
    end
  end

  def has_any_courses?
    current_user.courses.count
  end

################################
##       А вот это хорошо     ##
################################

  current_user.add_role(‘author’) if author?

  def author?
    current_user.id ==  course.user.id
  end

  def has_any_courses?
    current_user.courses.size > 0
  end
Явные условия с present? blank? nil?

Хорошим тоном считается всегда делать проверку методами present?, blank?, nil? и т.д. несмотря на то, что код в некоторых случаях и так будет работать.


################################
##       Это плохо!           ##
################################
  
  
  if course
   render '...'
  end

  raise NotAuthorized unless auth_token

################################
##       А вот это хорошо     ##
################################

  if course.present?
   render '...'
  end

  raise NotAuthorized unless auth_token.present?
Не используют мемоизацию

В ruby оператор || возвращает не будевое значение, а первый результат, который не nil. Инстанс переменная считается nil если не была объявлена. Таким образом, получается отличная смесь. Первый вариант выполнит 4ре запроса - 2 на выборку пользователя, один на выборку статей и один на выборку курсов, второй - только 3 запроса - 1 на выборку пользователя, один на выборку статей и один на выборку курсов.


################################
##       Это плохо!           ##
################################
  
  
  def user
    User.find(params[:id})
  end

  user.articles
  user.courses

################################
##       А вот это хорошо     ##
################################

  def user
    @user ||= User.find(params[:id})
  end

  user.articles
  user.courses
Не знают о соглашении "символ, потом одинарные кавычки, потом двойные кавычки"

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


################################
##       Это плохо!           ##
################################
  
  
  render ‘new’
  social_network = “facebook”
  message = “Can’t do this”


################################
##       А вот это хорошо     ##
################################

  render :new
  social_network = ‘facebook’
  message = “Can’t do this”
Используют магические числа и строки

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


################################
##       Это плохо!           ##
################################
  
  
  @courses = Course.recent.page(params[:page]).per(3)

  If team.players.count > 13
    # do something
  end

################################
##       А вот это хорошо     ##
################################

  @courses = Course.recent.page(params[:page]).per(params[:per_page] || PER_PAGE)

  if team.players.count > MAXIMUM_NUMBER_OF_PLAYERS
    # do something  
  end
Называют переменные и константы короткими и непонятными именами

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


################################
##       Это плохо!           ##
################################
  
  
  u = User.last

  CUS_NUM = 100

################################
##       А вот это хорошо     ##
################################

  last_user = User.last

  CUSTOMERS_NUMBER = 100

  # Но

  form_for @article do |f|
    = f.text_field :title


  create_table :articles do |t|
    t.integer :course_id
Злоупотребляют пост условиями

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

1 Когда оно теряется в строке
2 Когда это самое главное условие в методе - оно должно быть максимально заметным


################################
##       Это плохо!           ##
################################
  
  
  def some_method
    SomeLongClassName.new(params).perform_action if something_true?

    # a lot of another code
  end
  
  def create
    redirect_to(root_path) if @resource.save
  end

################################
##       А вот это хорошо     ##
################################

  def some_method
    if something_true?
      SomeLongClassName.new(params).perform_action
    end

    # a lot of another code
  end

  def create
    if @resource.save
      redirect_to root_path
    end
  end
Оставляют неиспользуемые параметры и переменные. Боятся удалять код, который больше не нужен.

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


################################
##       Это плохо!           ##
################################
  
  
  def find_user( first_name, last_name, date_of birth )
    User.find_by(first_name: first_name)
  end

################################
##       А вот это хорошо     ##
################################

  def find_user(first_name)
    User.find_by(first_name: first_name)
  end
Не используют public_send

Почти во всех примерах из интернета используют send, когда нужно вызвать метод динамически. Но, есть схожий метод, который вызывать предпочтительнее, если это возможно.

Пишут длинные и не читаемые условия

Длинные условия, особенно с unless, не читаются. Им можно дать читаемые псевдонимы, поместив их в методы.


################################
##       Это плохо!           ##
################################
  
  
  if course.courses_users.where(ban: true).exists?(user_id: id)
    if course.user_id ==   id
      if #something                                                                                                                                                                                                          course.part.exists?(self)
      end
    end
  end

################################
##       А вот это хорошо     ##
################################

  if part_in?(course) && banned?(course) && author?(course)
    # do something
  end

  def part_in?(course)
    course.part.exists?(self)
  end

  def banned?(course)
    course.courses_users.where(ban: true).exists?(user_id: id)
  end

  def author?(course)
    course.user_id == id
  end
Гоняются за количеством строк

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


################################
##       Это плохо!           ##
################################
  
  
  def some_filter
    return unless user_signed_in? || !(current_user.customer? || current_user.admin?)

    redirect_to root_path
  end

################################
##       А вот это хорошо     ##
################################

  def some_filter
    return unless user_signed_in? 
    return if current_user.customer? || current_user.admin?

    redirect_to root_path
  end
Используют устаревший синтаксис

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


################################
##       Это плохо!           ##
################################
  
  
  a = {:bad_code => ‘Some smell’ }

################################
##       А вот это хорошо     ##
################################

  a = { good_code: ‘No smells’ }
Злоупотребляют when в case или if операторами. 

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


################################
##       Это плохо!           ##
################################
  
  
  def somethod(name)
    case name
    when ‘a’
      ‘A’
    when ‘b’
      ‘B’
    when '…'
      '...'
    end
  end  

################################
##       А вот это хорошо     ##
################################

  CASES = {
   ‘a’ => ‘A’,
   ‘b’ => ‘B’,
  }
  
  def somethod(name)
    CASES[name]
  end

  # или такой вариант

  def somethod(name)
    send(:”process_#{name}”)
  end

  def process_a
   ‘A’
  end

  def process_b
   ‘B’
  end
Не используют rubocop и другие анализаторы качества кода.

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

Как это обычно бывает.

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

А как хотелось бы, но эх...

Я начинающий программист. Я прочитал ruby style guide, но еще не все запомнил и всегда пользуюсь рубокопом, чтобы не пропустить банальных ошибок. Мой наставник отметил, что моя работа принимается быстрее и содержит меньше всего комментариев и выбрал меня среди всех джуниоров на новый, сложный проект

Неверно понимают смысл комментирования кода

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

Мюсли почти каждого новичка.

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

Мюсли новичка, который читал Макконела=)

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

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

Продолжение следует!

Комментарии