jeudi 15 juin 2017

Ruby good practices, patterns. Critics and good points

I'm a junior Ruby dev and in a recruitment process I was told they are not looking for a junior but sometime some beginner can be a good surpise so they ask me to do the following test :

  • Have a look on the folowing piece of code (below) tell us whats pertinent/good for you, and what points seem irrelevant to you, give us your remarks with technical explanations.

  • Send us a corrected version of this code, applying the above remarks.

I'm not asking the community to do my job of course. But i think asking the communitys opinion and advices is one of the important qualities for a dev so i'm doing it ! What do you think about this code ? What are the good pattern used and what pattern could i use to improve it ? Thanks

class Car < ActiveRecord::Base
  attr_accessible :color, :brand
  belongs_to :user

  def self.count_by_colors
    rows = connection.execute(<<-SQL)
      SELECT color AS COLOR, COUNT(*) AS count
      FROM cars
      GROUP BY color
    SQL
    rows.each_with_object({}) { |acc, row| acc[row['color']] = row['count'] }
  end
end

class Organisation < ActiveRecord::Base
  has_many :users

end

class User < ActiveRecord::Base
  attr_accessible :first_name, :last_name
  has_many :cars

  def self.users_with_red_cars
    car_ids = Car.where(color: 'red').pluck(:id)
    User.joins(:cars).where(cars: { id: car_ids }).to_a
  end
end

class UserController < ActionController::Base

  def show
    @user = User.where("id = #{params[:id]}").to_a.first
    render @user
  end

  def print_users_with_red_cars
    User.users_with_red_cars.each do |user|
      puts user.first_name + ' ' + user.last_name + ' has a red car'
    end
  end

  def red_brands
    results = []
    User.users_with_red_cars.each do |user|
      user.cars.each { |car| results << car.brand if car.color == 'red' && !results.include?(car.brand) }
    end
    render json: results.to_json
  end
end

Aucun commentaire:

Enregistrer un commentaire