Refactoring some Ruby Code

Question

I need some help refactoring some ruby code. Im not keeping it DRY at all.

if potatoes
  if item.type != nil
    if item.has_stuff == false && (item.something_else).to_f >= (comparing).to_f
      # RUN JOB A
    else
      # RUN JOB B
    end
  else
    # RUN JOB A
  end
else
  # RUN JOB B
end

I just created random names for things.

@tadman You mean "unless it's possible that x might be false". — sawa
– sawa, Commented Feb 24, 2015 at 17:47
You might want to take a look at Code Review Stack Exchange. I haven't been over there in a while but this sounds like their type of question. — Charles Caldwell
– Charles Caldwell, Commented Feb 24, 2015 at 17:47
@CharlesCaldwell as it currently stands, migration would be rejected for being hypothetical or example code. — Mathieu Guindon
– Mathieu Guindon, Commented Feb 24, 2015 at 17:50
@theTinMan This is example code and would be off-topic on Code Review. Please read Be careful when recommending Code Review to askers — Simon Forsberg
– Simon Forsberg, Commented Feb 24, 2015 at 18:08

Max · Accepted Answer · 2015-02-24 17:41:53Z

2

if potatoes && (item.type.nil? || (item.has_stuff == false && (item.something_else).to_f >= (comparing).to_f))
  # JOB A
else
  # JOB B
end

But for such complicated logic, it might be better to pull part of that into a method

def item.has_some_property?(comparing)
  has_stuff == false && something_else.to_f >= comparing.to_f
end

if potatoes && (item.type.nil? || item.has_some_property?(comparing))
  # JOB A
else
  # JOB B
end

answered Feb 24, 2015 at 17:41

Max

22.5k7 gold badges55 silver badges79 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

sawa · Accepted Answer · 2015-02-24 17:40:04Z

1

if !potatoes
  # Job B
elsif item.type.nil?
  # Job A
elsif item.has_stuff != false
  # Job B
elsif item.something_else.to_f >= comparing.to_f
  # Job A
else
  # Job B
end

answered Feb 24, 2015 at 17:40

sawa

169k51 gold badges287 silver badges398 bronze badges

Comments

Mauro P. · Accepted Answer · 2015-02-24 18:22:56Z

If you associate the jobs to external methods (defined elsewhere) you can keep it dry by using ternary operators, and using a different perspective, considering that the conditions are complementary... to keep the code more readable you can write the conditions on different lines, if you wish, or you should use a method to perform the checks (i.e. item.has_stuff == false && (item.something_else).to_f >= (comparing).to_f) if meaningful in your code...

A quick example:

def is_empty_and_something_less_than_something?(comparing)
   item.has_stuff == false && (item.something_else).to_f >= (comparing).to_f
end

job_a_conditions= potatoes && (item.type.nil? || item.is_empty_and_something_less_than_comparing?(comparing))
job_a_conditions ? job_a : job_b

The same conditions can be associated to a method, if re-used in your code.

Collectives™ on Stack Overflow

Refactoring some Ruby Code

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related