Avatar

Can anyone help refactor this disgusting loop?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
		for feedback in @feedbacks
			unless feedback.question1.nil?
				data[0][feedback.question1.to_i - 1] += 1
			end
			unless feedback.question2.nil?
				data[1][feedback.question2.to_i - 1] += 1
			end
			unless feedback.question3.nil?
				data[2][feedback.question3.to_i - 1] += 1
			end
			unless feedback.question4.nil?
				data[3][feedback.question4.to_i - 1] += 1
			end
			unless feedback.question5.nil?
				data[4][feedback.question5.to_i - 1] += 1
			end
			unless feedback.question6.nil?
				data[5][feedback.question6.to_i - 1] += 1
			end
			unless feedback.question7.nil?
				data[6][feedback.question7.to_i - 1] += 1
			end
			unless feedback.question8.nil?
				data[7][feedback.question8.to_i - 1] += 1
			end
			unless feedback.question9.nil?
				data[8][feedback.question9.to_i - 1] += 1
			end
			unless feedback.question10.nil?
				data[9][feedback.question10.to_i - 1] += 1
			end
			unless feedback.question11.nil?
				data[10][feedback.question11.to_i - 1] += 1
			end
			unless feedback.question12.nil?
				data[11][feedback.question12.to_i - 1] += 1
			end
			unless feedback.question13.nil?
				data[12][feedback.question13.to_i - 1] += 1
			end
			unless feedback.question14.nil?
				data[13][feedback.question14.to_i - 1] += 1
			end
			unless feedback.question15.nil?
				data[14][feedback.question15.to_i - 1] += 1
			end
			unless feedback.question16.nil?
				data[15][feedback.question16.to_i - 1] += 1
			end
			unless feedback.question17.nil?
				data[16][feedback.question17.to_i - 1] += 1
			end
			unless feedback.question18.nil?
				data[17][feedback.question18.to_i - 1] += 1
			end
			unless feedback.question19.nil?
				data[18][feedback.question19.to_i - 1] += 1
			end
			unless feedback.question20.nil?
				data[19][feedback.question20.to_i - 1] += 1
			end
		end

Refactorings

No refactoring yet !

8bbc5f91ed42b51a7053788d5096465c

Ingemar

September 17, 2008, September 17, 2008 10:20, permalink

1 rating. Login to rate!

Line three can definitely be refactored more but since I don't know what ".to_i" will produce I can't do it right now.

1
2
3
4
5
@feedbacks.each_with_index do |feedback, i|
  unless feedback.send("question#{i+1}").nil?
    data[i][feedback.send("question#{i+1}").to_i - 1] += 1
  end
end
Avatar

toggo.myopenid.com

September 17, 2008, September 17, 2008 10:40, permalink

No rating. Login to rate!

settled for this in the end. thank you for your help :)

1
2
3
4
5
6
7
		for feedback in @feedbacks
			0.upto(19) do |i|
				unless feedback.send("question#{i+1}").nil?
					data[i][feedback.send("question#{i+1}").to_i - 1] += 1
				end
			end
		end
8bbc5f91ed42b51a7053788d5096465c

Ingemar

September 17, 2008, September 17, 2008 10:44, permalink

No rating. Login to rate!

I think you're doing something wrong here, if you need a loop like this.
What are you trying achieve anyway?

880cbab435f00197613c9cc2065b4f5a

danielharan

September 17, 2008, September 17, 2008 14:29, permalink

No rating. Login to rate!

Trying to remove more duplication. I'm fairly certain there's an easier way to do what you want, so like Ingemar have to ask what you are trying to achieve.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
for feedback in @feedbacks
  1.upto(20) do |i|
    # unless (answer = feedback.question(i)).nil? # lets you avoid a second call
    unless feedback.question(i).nil?
       # answer.to_i - 1 # is also suspect. why not just store data[i][answer] ?
      data[i][feedback.question(i).to_i - 1] += 1
    end
  end
end

class Feedback
  def question(id)
    send("question#{id}")
  end
end
A8d3f35baafdaea851914b17dae9e1fc

Adam

September 17, 2008, September 17, 2008 16:34, permalink

1 rating. Login to rate!
1
2
3
4
5
6
@feedbacks.each do |feedback|
  data.each do |key,value|
    question = feedback.send("question#{key + 1}")
    value[question.to_i - 1] += 1 if question
  end
end

Your refactoring





Format Copy from initial code

or Cancel