Ruby on Rails - Самые распространенные ошибки новичка. Часть 2 - Контроллеры

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

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

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

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

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

Привыкли использовать инстанс переменные в контроллерах

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


################################
##       Это плохо!           ##
################################
  
  class API::ArticlesController
   def index
     @articles = Article.all
     render json: @articles, status: 200
   end
  end

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

  class API::ArticlesController
   def index
     articles = Article.all
     render json: articles, status: 200
   end
  end

Не указывают HTTP статус ответа в API контроллерах

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


################################
##       Это плохо!           ##
################################
  
  class API::ArticlesController
    def create
      article = Article.new(article_params)
      
      if article.save
        render json: article
      else
        render json: { errors: article.errors.full_messages }
      end  
    end
  end

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

  class API::ArticlesController
    def create
      article = Article.new(article_params)
      
      if article.save
        render json: article, status: 200
      else
        render json: { errors: article.errors.full_messages }, status: 422
      end  
    end
  end

Инициализируют или ищут объекты без связи с родителем

В контроллере очень часто доступен родитель для текущего объекта. Например, в личном кабинете пользователь должен редактировать только свои объекты. Уроки, должны искаться только в контексте курса.
Использование этой простой практики повышает стабильность приложения и немного его безопасность. Я даже не могу описать, насколько этот подход качественнее и скольких ошибок он помогает избегать. Кстати у меня пример с личным кабинетом, но про этот же подход не стоит забывать, и когда Вы просто используете nested routes. Нашли родителя, а уже от него - дочерний объект.


################################
##       Это плохо!           ##
################################
  
  
  class Users::CoursesController
    def edit
      @course = Course.find(params[:id])
    end
  end

  class Users::LessonsController
    def new
      @lesson = Lesson.new(lesson_params)
    end
  end

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

  class Users::CoursesController
    def edit
      @course = current_user.courses.find(params[:id])
    end
  end

  class Users::LessonsController
    def new
      course = current_user.courses.find(params[:course_id])
      @lesson = course.lessons.build(lesson_params)
    end
  end

Используют инстанс переменные в паршиалах

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


################################
##       Это плохо!           ##
################################
  
  
  = render 'form'

  # _form.html.haml:
  = simple_form_for [:users, @course] do |f|
    - # some form

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

  = render 'form', course: course

  # _form.html.haml:
  = simple_form_for [:users, course] do |f|
    - # some form

Не используют before_action

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


################################
##       Это плохо!           ##
################################
  
  
  class Users::CourseController
    def update
      if current_user.has_role?(‘trainer’)
        @course = current_user.courses.find(params[:id])

        if @course.save
          redirect_to courses_path
        else
          render :edit
        end
      else
        redirect_to root_path, notice: ‘You are not authorized’
      end
    end
  end

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

  class Users::CourseController
    before_action :ensure_user_has_access
    before_action :find_course

    def update
      if @course.save
        redirect_to courses_path
      else
        render :edit
      end
    end

    private

    def ensure_user_has_access
      unless current_user.has_role?(‘trainer’)
        redirect_to root_path, notice: ‘You are not authorized’
      end 
    end

    def find_course
      @course = current_user.courses.find(params[:id])
    end
  end

Не пользуются хелперами

Хелперы позволяют очень сильно упростить себе жизнь. Например, в приложении очень часто приходится использовать remote формы. Можно дописывать этот параметр к каждой форме, а можно...


################################
##   Упрощаем себе жизнь      ##
################################

  module RemoteFormHelper
    def remote_form(path, &block)
      simple_form_for path, remote: true  do |f|
        yield f
        concat f.submit 'Save',       class: 'ui save green button'
        concat link_to 'Cancel', '#', class: 'ui cancel button'
      end
    end
  end

Второе применение хелперов - сокрытие логики из вьюх

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

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