Commit d90fe8cd2196985384b873e656404a0bef56d929
1 parent
4c7f9366
Exists in
master
and in
1 other branch
make updates to simple random choose prompt
rename picked_prompt to simple_random_choose_prompt add tests for distinct_array_of_choice_ids modify distinct_array_of_choice_ids to use named parameters move conditions variable creation outside of loop update conditions creation to handle when only_active is false
Showing
2 changed files
with
35 additions
and
12 deletions
Show diff stats
app/models/question.rb
@@ -57,7 +57,7 @@ class Question < ActiveRecord::Base | @@ -57,7 +57,7 @@ class Question < ActiveRecord::Base | ||
57 | next_prompt = self.pop_prompt_queue | 57 | next_prompt = self.pop_prompt_queue |
58 | if next_prompt.nil? | 58 | if next_prompt.nil? |
59 | logger.info("DEBUG Catchup prompt cache miss! Nothing in prompt_queue") | 59 | logger.info("DEBUG Catchup prompt cache miss! Nothing in prompt_queue") |
60 | - next_prompt = self.picked_prompt | 60 | + next_prompt = self.simple_random_choose_prompt |
61 | record_prompt_cache_miss | 61 | record_prompt_cache_miss |
62 | else | 62 | else |
63 | record_prompt_cache_hit | 63 | record_prompt_cache_hit |
@@ -66,16 +66,16 @@ class Question < ActiveRecord::Base | @@ -66,16 +66,16 @@ class Question < ActiveRecord::Base | ||
66 | return next_prompt | 66 | return next_prompt |
67 | else | 67 | else |
68 | #Standard choose prompt at random | 68 | #Standard choose prompt at random |
69 | - return self.picked_prompt | 69 | + return self.simple_random_choose_prompt |
70 | end | 70 | end |
71 | 71 | ||
72 | end | 72 | end |
73 | 73 | ||
74 | #TODO: generalize for prompts of rank > 2 | 74 | #TODO: generalize for prompts of rank > 2 |
75 | - def picked_prompt(rank = 2) | ||
76 | - logger.info "inside Question#picked_prompt" | 75 | + def simple_random_choose_prompt(rank = 2) |
76 | + logger.info "inside Question#simple_random_choose_prompt" | ||
77 | raise NotImplementedError.new("Sorry, we currently only support pairwise prompts. Rank of the prompt must be 2.") unless rank == 2 | 77 | raise NotImplementedError.new("Sorry, we currently only support pairwise prompts. Rank of the prompt must be 2.") unless rank == 2 |
78 | - choice_id_array = distinct_array_of_choice_ids(rank, true) | 78 | + choice_id_array = distinct_array_of_choice_ids(:rank => rank, :only_active => true) |
79 | prompts.find_or_create_by_left_choice_id_and_right_choice_id(choice_id_array[0], choice_id_array[1], :include => [:left_choice ,:right_choice ]) | 79 | prompts.find_or_create_by_left_choice_id_and_right_choice_id(choice_id_array[0], choice_id_array[1], :include => [:left_choice ,:right_choice ]) |
80 | end | 80 | end |
81 | 81 | ||
@@ -334,13 +334,20 @@ class Question < ActiveRecord::Base | @@ -334,13 +334,20 @@ class Question < ActiveRecord::Base | ||
334 | end | 334 | end |
335 | 335 | ||
336 | 336 | ||
337 | - def distinct_array_of_choice_ids(rank = 2, only_active = true) | 337 | + def distinct_array_of_choice_ids(params={}) |
338 | + params = { | ||
339 | + :rank => 2, | ||
340 | + :only_active => true | ||
341 | + }.merge(params) | ||
342 | + rank = params[:rank] | ||
343 | + only_active = params[:only_active] | ||
338 | count = (only_active) ? choices.active.count : choices.count | 344 | count = (only_active) ? choices.active.count : choices.count |
339 | 345 | ||
340 | found_choices = [] | 346 | found_choices = [] |
347 | + # select only active choices? | ||
348 | + conditions = (only_active) ? ['active = ?', true] : ['1=1'] | ||
349 | + | ||
341 | rank.times do | 350 | rank.times do |
342 | - # select only active choices? | ||
343 | - conditions = (only_active) ? ['active = ?', true] : [''] | ||
344 | # if we've already found some, make sure we don't find them again | 351 | # if we've already found some, make sure we don't find them again |
345 | if found_choices.count > 0 | 352 | if found_choices.count > 0 |
346 | conditions[0] += ' AND id NOT IN (?)' | 353 | conditions[0] += ' AND id NOT IN (?)' |
@@ -350,13 +357,14 @@ class Question < ActiveRecord::Base | @@ -350,13 +357,14 @@ class Question < ActiveRecord::Base | ||
350 | found_choices.push choices.find(:first, | 357 | found_choices.push choices.find(:first, |
351 | :select => 'id', | 358 | :select => 'id', |
352 | :conditions => conditions, | 359 | :conditions => conditions, |
360 | + # rand generates value >= 0 and < param | ||
353 | :offset => rand(count - found_choices.count)).id | 361 | :offset => rand(count - found_choices.count)).id |
354 | end | 362 | end |
355 | return found_choices | 363 | return found_choices |
356 | end | 364 | end |
357 | 365 | ||
358 | def picked_prompt_id | 366 | def picked_prompt_id |
359 | - picked_prompt.id | 367 | + simple_random_choose_prompt.id |
360 | end | 368 | end |
361 | 369 | ||
362 | def self.voted_on_by(u) | 370 | def self.voted_on_by(u) |
spec/models/question_spec.rb
@@ -37,10 +37,23 @@ describe Question do | @@ -37,10 +37,23 @@ describe Question do | ||
37 | #end | 37 | #end |
38 | 38 | ||
39 | it "should choose an active prompt randomly" do | 39 | it "should choose an active prompt randomly" do |
40 | - prompt = @question.picked_prompt | 40 | + prompt = @question.simple_random_choose_prompt |
41 | prompt.active?.should == true | 41 | prompt.active?.should == true |
42 | end | 42 | end |
43 | 43 | ||
44 | + it "should randomly choose two active choices" do | ||
45 | + 50.times do | ||
46 | + choice_ids = @question.distinct_array_of_choice_ids(:rank => 2, :only_active => true) | ||
47 | + choice_ids.count.should == 2 | ||
48 | + choice_ids.uniq.count.should == 2 | ||
49 | + choice_ids.each do |choice_id| | ||
50 | + c = Choice.find(choice_id) | ||
51 | + c.active?.should == true | ||
52 | + end | ||
53 | + end | ||
54 | + | ||
55 | + end | ||
56 | + | ||
44 | it "should choose an active prompt using catchup algorithm" do | 57 | it "should choose an active prompt using catchup algorithm" do |
45 | prompt = @question.catchup_choose_prompt | 58 | prompt = @question.catchup_choose_prompt |
46 | prompt.active?.should == true | 59 | prompt.active?.should == true |
@@ -321,7 +334,8 @@ describe Question do | @@ -321,7 +334,8 @@ describe Question do | ||
321 | end | 334 | end |
322 | 335 | ||
323 | 200.times.each do |num| | 336 | 200.times.each do |num| |
324 | - @p = @aoi_question.picked_prompt | 337 | + @p = @aoi_question.simple_random_choose_prompt |
338 | + @p.active?.should == true | ||
325 | 339 | ||
326 | @a = user.record_appearance(visitor, @p) | 340 | @a = user.record_appearance(visitor, @p) |
327 | 341 | ||
@@ -454,7 +468,8 @@ describe Question do | @@ -454,7 +468,8 @@ describe Question do | ||
454 | {:data => "foo'bar", :local_identifier => "example creator"}) | 468 | {:data => "foo'bar", :local_identifier => "example creator"}) |
455 | 469 | ||
456 | 40.times.each do |num| | 470 | 40.times.each do |num| |
457 | - @p = @aoi_question.picked_prompt | 471 | + @p = @aoi_question.simple_random_choose_prompt |
472 | + @p.active?.should == true | ||
458 | 473 | ||
459 | @a = user.record_appearance(visitor, @p) | 474 | @a = user.record_appearance(visitor, @p) |
460 | 475 |