CODEWITHSUNDEEP
ruby-on-railsrubyrefactoringruby-on-rails-5
marcamillion
marcamillion

Reputation: 33795

How do I refactor this to be much more DRY?

Basically what I am doing here is keeping track of 4 different types of ratings. If any rating is present, I would like to apply a different css class to that rating and then apply a default css class to the other spans rendered in that line.

ratings = ["speed", "tackling", "passing", "dribbling"]
ratings.each do |rating|
  content_tag :div, class: "col-lg-3" do
    if rating_param.eql? rating
      if rating.eql? "speed"
        content_tag :span, class: "label label-success label-lg" do
          "#{rating.capitalize}: #{profile.ratings.find_by(user: current_user)[rating]}"
        end
      elsif rating.eql? "tackling"
        content_tag :span, class: "label label-tackling label-lg" do
          "#{rating.capitalize}: #{profile.ratings.find_by(user: current_user)[rating]}"
        end
      end
    else
      content_tag :span, class: "label label-default" do
        "#{rating.capitalize}: #{profile.ratings.find_by(user: current_user)[rating]}"
      end
    end
  end
end

So basically what I need to do is map the rating to the class.

For instance, it may look like this:

speed: success, tackling: info, dribbling: primary, passing: warning.

So if the rating is speed, it should apply the class success, and so on.

How do I refactor this so it isn't a bunch of ugly if statements like this?

Upvotes: 1

Views: 88

Answers (2)

Amit Sharma
Amit Sharma

Reputation: 3477

You can also try this..

rating_classes = {
  "speed"     => "label label-success label-lg",
  "tackling"  => "label label-tackling label-lg",
  "passing"   => "label label-default",
  "dribbling" => "label label-default"
}

profile_rating = profile.ratings.find_by(user: current_user)

content_tag :div, class: "col-lg-3" do
  rating_classes.each do |rating, klass|
    content_tag :span, class: "#{klass}" do
      "#{rating.capitalize}: #{profile_rating[rating]}"
    end
  end
end

You have written profile.ratings.find_by(user: current_user) in loop which executes same query 4 times which may reduce performance.

Upvotes: 0

Dave Newton
Dave Newton

Reputation: 160321

(Not a complete answer, but useless in a comment.)

(Very) roughly:

rating_classes = {
  'speed'     => 'label-success label-lg',
  'tackling'  => 'label-tackling label-lg',
  'passing'   => 'whatever',
  'dribbling' => 'whatever'
}

rating_class = rating_classes[rating_param] || 'label-default'

content_tag :div, class: "col-lg-3" do
  content_tag :span, class: "label #{rating_class}" do
    "#{rating.capitalize}: #{profile.ratings.find_by(user: current_user)[rating]}"
  end
end

Upvotes: 2

Related Questions