CODEWITHSUNDEEP
ruby-on-railsloopscontroller
Jordan
Jordan

Reputation: 1457

What's a better way of writing this loop?

Any thoughts on how to build a better loop for this rails controller?

I get a little lost on logic loops sometimes, so I resort to neanderthal code.

def index

 @step1_status = current_user.steps.pluck(:step1).first
 @step2_status = current_user.steps.pluck(:step2).first
 @step3_status = current_user.steps.pluck(:step3).first
 @step4_status = current_user.steps.pluck(:step4).first
 @step5_status = current_user.steps.pluck(:step5).first
 @step6_status = current_user.steps.pluck(:step6).first
 @step7_status = current_user.steps.pluck(:step7).first
 @step8_status = current_user.steps.pluck(:step8).first
 @step9_status = current_user.steps.pluck(:step9).first

 if @step9_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => ['1', '2', '3', '4', '5', '6', '7', '8', '9'])
 elsif @step8_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => ['1', '2', '3', '4', '5', '6', '7', '8'])
 elsif @step7_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => ['1', '2', '3', '4', '5', '6', '7'])
 elsif @step6_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => ['1', '2', '3', '4', '5', '6'])
 elsif @step5_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => ['1', '2', '3', '4', '5'])
 elsif @step4_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => ['1', '2', '3', '4'])
 elsif @step3_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => ['1', '2', '3'])
 elsif @step2_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => ['1', '2'])
 elsif @step1_status == true
  @task = Task.limit(1).order('sort_id ASC').where.not(:sort_id => '1')
 else
  @task = Task.limit(1).order('sort_id ASC')
 end

end

Thanks for your help!

Upvotes: 1

Views: 46

Answers (1)

Michael Gorman
Michael Gorman

Reputation: 1077

I think this should be the same.

First try

@step_status = []
9.times do |n|
  @step_status << current_user.steps.pluck("step#{n+1}").first
end

highest_step = @step_status.rindex(true)
if highest_step
  @task = Task.order('sort_id ASC').where.not(sort_id: (1..(highest_step+1))).first
else
  @task = Task.order('sort_id ASC').first
end

A bit more optimized and less code

highest_step = (1..9).to_a.rindex{|n| current_user.steps.pluck("step#{n+1}").first} + 1

@task = Task.order('sort_id ASC')
@task = @task.where.not(sort_id: (1..(highest_step))) if highest_step
@task = @task.first

Only 2 DB calls

pluck_array = (1..9).map{|n| "step#{n}"}
highest_step = current_user.steps.pluck(pluck_array).first.rindex(true) + 1 
#the + 1 accounts for the 1 being at index 0 through 9 at index 8

@task = Task.order('sort_id ASC')
@task = @task.where.not(sort_id: (1..(highest_step))) if highest_step
@task = @task.first

A bit easier to read (though a bit longer)

step_array = (1..9).to_a
pluck_array = step_array.map{|n| "step#{n}"}
highest_index = current_user.steps.pluck(pluck_array).first.rindex(true)
highest_step = step_array[highest_index]

@task = Task.order('sort_id ASC')
@task = @task.where.not(sort_id: (1..(highest_step))) if highest_step
@task = @task.first

Upvotes: 1

Related Questions