replaced choice#votes_count/loss_count with wins/losses

Dmitri Garbuzov
1 parent 5e0f9114
Showing 8 changed files with 46 additions and 69 deletions Show diff stats
app/controllers/choices_controller.rb
app/models/choice.rb
app/models/vote.rb
db/migrate/20100727203816_remove_wins_losses_and_rename_counts.rb
db/schema.rb
spec/models/choice_spec.rb
spec/models/question_spec.rb
spec/models/vote_spec.rb
@@ -34,15 +34,6 @@ class ChoicesController &lt; InheritedResources::Base
   end
-  def show
-    show! do |format|
-      format.xml { 
-        @choice.reload
-        render :xml => @choice.to_xml(:methods => [:wins_plus_losses])}
-      format.json { render :json => @choice.to_json(:methods => [:data])}
-    end 
-  end
-
   def votes
     @choice = Choice.find(params[:id])
     render :xml => @choice.votes.to_xml
@@ -16,39 +16,11 @@ class Choice &lt; ActiveRecord::Base
   after_save :update_questions_counter
-  attr_protected :prompts_count, :votes_count, :loss_count, :wins, :losses, :score,
-                 :prompts_on_the_right_count, :prompts_on_the_left_count
+  attr_protected :prompts_count, :wins, :losses, :score, :prompts_on_the_right_count, :prompts_on_the_left_count
   def update_questions_counter
     self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length)
   end 
-  #attr_accessor :data
-  
-  def lose!
-    Choice.increment_counter(:loss_count, self.id)
-    self.loss_count +=1 # reflect the update just done above, so score is correct
-    self.score = compute_score
-    self.save
-  end
-  
-  def win!
-    self.votes_count += 1 rescue (self.votes_count = 1)
-    self.score = compute_score
-    self.save!
-  end
-  
-  def wins_plus_losses
-    wins + losses
-  end
-  
-  # TODO delete these and refactor loss_count and votes_count into losses and wins
-  def losses
-    loss_count || 0
-  end
-  
-  def wins
-    votes_count || 0
-  end
   def before_create
     unless self.score
@@ -9,8 +9,8 @@ class Vote &lt; ActiveRecord::Base
   belongs_to :voter, :class_name => "Visitor", :foreign_key => "voter_id"
   belongs_to :question, :counter_cache => true
   belongs_to :prompt, :counter_cache => true
-  belongs_to :choice, :counter_cache => true
-  belongs_to :loser_choice, :class_name => "Choice", :foreign_key => "loser_choice_id"
+  belongs_to :choice, :counter_cache => true, :counter_cache => :wins
+  belongs_to :loser_choice, :class_name => "Choice", :foreign_key => "loser_choice_id", :counter_cache => :losses
   has_one :appearance, :as => :answerable
   named_scope :recent, lambda { |*args| {:conditions => ["created_at > ?", (args.first || Date.today.beginning_of_day)]} }
@@ -25,10 +25,12 @@ class Vote &lt; ActiveRecord::Base
   after_create :update_winner_choice, :update_loser_choice
   def update_winner_choice
-     choice.win!
+    choice.reload              # make sure we're using updated counter values
+    choice.compute_score!
   end
   def update_loser_choice 
-     loser_choice.lose!
+    loser_choice.reload
+    loser_choice.compute_score!
   end
 end
@@ -0,0 +1,15 @@
+class RemoveWinsLossesAndRenameCounts < ActiveRecord::Migration
+  def self.up
+    remove_column :choices, :wins
+    remove_column :choices, :losses
+    rename_column :choices, :votes_count, :wins
+    rename_column :choices, :loss_count, :losses
+  end
+
+  def self.down
+    rename_column :choices, :wins, :votes_count
+    rename_column :choices, :losses, :loss_count
+    add_column :choices, :wins, :integer
+    add_column :choices, :losses, :integer
+  end
+end
@@ -9,7 +9,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20100722052215) do
+ActiveRecord::Schema.define(:version => 20100727203816) do
   create_table "appearances", :force => true do |t|
     t.integer  "voter_id"
@@ -32,9 +32,7 @@ ActiveRecord::Schema.define(:version =&gt; 20100722052215) do
     t.integer  "item_id"
     t.integer  "question_id"
     t.integer  "position"
-    t.integer  "wins"
     t.integer  "ratings"
-    t.integer  "losses"
     t.datetime "created_at"
     t.datetime "updated_at"
     t.integer  "request_id"
@@ -45,8 +43,8 @@ ActiveRecord::Schema.define(:version =&gt; 20100722052215) do
     t.string   "local_identifier"
     t.integer  "prompts_on_the_left_count",  :default => 0
     t.integer  "prompts_on_the_right_count", :default => 0
-    t.integer  "votes_count",                :default => 0
-    t.integer  "loss_count",                 :default => 0
+    t.integer  "wins",                       :default => 0
+    t.integer  "losses",                     :default => 0
     t.integer  "prompts_count",              :default => 0
     t.string   "data"
     t.integer  "creator_id"
@@ -27,8 +27,6 @@ describe Choice do
     @unreasonable_value = 9999
     @protected_attributes = {}
     [ :prompts_count,
-      :votes_count,
-      :loss_count,
       :wins,
       :losses,
       :score,
@@ -100,15 +98,15 @@ describe Choice do
   end
   it "correctly compute a score based on wins and losses" do 
           choice1 = Choice.create!(@valid_attributes)
-	  choice1.votes_count = 30
-	  choice1.loss_count = 70
+	  choice1.wins = 30
+	  choice1.losses = 70
 	  choice1.compute_score.should be_close(30,1)
   end
   it "compute score and save" do 
           choice1 = Choice.create!(@valid_attributes)
 	  choice1.score.should == 50
-	  choice1.votes_count= 30
-	  choice1.loss_count = 70
+	  choice1.wins = 30
+	  choice1.losses = 70
 	  choice1.compute_score!
 	  choice1.score.should be_close(30, 1)
   end
@@ -124,15 +122,19 @@ describe Choice do
   end
   describe "voting updates things" do
-     before do
-          @prompt = @question.choose_prompt
-          @winning_choice = @prompt.left_choice
-	  @losing_choice = @prompt.right_choice
-	  @winning_choice.win!
-	  @losing_choice.lose!
+    before do
+      @prompt = @question.choose_prompt
+      @winning_choice = @prompt.left_choice
+      @losing_choice = @prompt.right_choice
+      vote = Vote.create!(:choice_id => @winning_choice.id,
+                          :loser_choice_id => @losing_choice.id,
+                          :question_id => @question.id,
+                          :voter_id => @visitor.id,
+                          :prompt_id => @prompt.id )
      end
      it "should update score on a win" do
+	  @winning_choice.reload
           @winning_choice.score.should be_close(67, 1)
      end
      it "should update score on a loss" do
@@ -140,15 +142,13 @@ describe Choice do
           @losing_choice.score.should be_close(33,1)
      end
      it "should update win count on a win" do
-          @winning_choice.votes_count.should == 1
+	  @winning_choice.reload
           @winning_choice.wins.should == 1
-          @winning_choice.loss_count.should == 0
           @winning_choice.losses.should == 0
      end
      it "should update loss count on a loss" do
-          @losing_choice.votes_count.should == 0
+	  @losing_choice.reload
           @losing_choice.wins.should == 0
-          @losing_choice.loss_count.should == 1
           @losing_choice.losses.should == 1
      end
   end
@@ -173,7 +173,7 @@ describe Question do
   it "should provide average voter information" do
     params = {:id => 124, :visitor_identifier => "jim", :with_prompt => true, :with_appearance => true, :with_average_votes => true }
     @question_optional_information = @question.get_optional_information(params)
-    @question_optional_information[:average_votes].should be_an_instance_of(Float)
+    @question_optional_information[:average_votes].should be_an_instance_of(Fixnum)
     @question_optional_information[:average_votes].should be_close(0.0, 0.1)
     vote_options = {:visitor_identifier => "jim",
@@ -38,18 +38,18 @@ describe Vote do
   end
   it "should update counter cache on choice" do
     @prompt.left_choice.votes.size.should == 0
-    @prompt.left_choice.votes_count.should == 0
+    @prompt.left_choice.wins.should == 0
     Factory.create(:vote, :question => @question, :prompt => @prompt, 
                           :choice => @prompt.left_choice)
     @prompt.left_choice.reload
     @prompt.left_choice.votes.size.should == 1
-    @prompt.left_choice.votes_count.should == 1
+    @prompt.left_choice.wins.should == 1
   end
   it "should update counter cache on loser_choice" do
     @prompt.left_choice.votes.size.should == 0
     @prompt.right_choice.losses.should == 0
-    @prompt.left_choice.votes_count.should == 0
+    @prompt.left_choice.wins.should == 0
     Factory.create(:vote, :question => @question, :prompt => @prompt,
                           :choice => @prompt.left_choice,
                           :loser_choice => @prompt.right_choice)
@@ -57,8 +57,7 @@ describe Vote do
     @prompt.right_choice.reload
     @prompt.right_choice.votes.size.should == 0
-    @prompt.right_choice.votes_count.should == 0
-    @prompt.right_choice.loss_count.should == 1
+    @prompt.right_choice.wins.should == 0
     @prompt.right_choice.losses.should == 1
   end
	@@ -34,15 +34,6 @@ class ChoicesController < InheritedResources::Base		@@ -34,15 +34,6 @@ class ChoicesController < InheritedResources::Base
34		34
35	end	35	end
36		36
37	- def show
38	- show! do \|format\|
39	- format.xml {
40	- @choice.reload
41	- render :xml => @choice.to_xml(:methods => [:wins_plus_losses])}
42	- format.json { render :json => @choice.to_json(:methods => [:data])}
43	- end
44	- end
45	-
46	def votes	37	def votes
47	@choice = Choice.find(params[:id])	38	@choice = Choice.find(params[:id])
48	render :xml => @choice.votes.to_xml	39	render :xml => @choice.votes.to_xml
	@@ -16,39 +16,11 @@ class Choice < ActiveRecord::Base		@@ -16,39 +16,11 @@ class Choice < ActiveRecord::Base
16		16
17	after_save :update_questions_counter	17	after_save :update_questions_counter
18		18
19	- attr_protected :prompts_count, :votes_count, :loss_count, :wins, :losses, :score,
20	- :prompts_on_the_right_count, :prompts_on_the_left_count	19	+ attr_protected :prompts_count, :wins, :losses, :score, :prompts_on_the_right_count, :prompts_on_the_left_count
21		20
22	def update_questions_counter	21	def update_questions_counter
23	self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length)	22	self.question.update_attribute(:inactive_choices_count, self.question.choices.inactive.length)
24	end	23	end
25	- #attr_accessor :data
26	-
27	- def lose!
28	- Choice.increment_counter(:loss_count, self.id)
29	- self.loss_count +=1 # reflect the update just done above, so score is correct
30	- self.score = compute_score
31	- self.save
32	- end
33	-
34	- def win!
35	- self.votes_count += 1 rescue (self.votes_count = 1)
36	- self.score = compute_score
37	- self.save!
38	- end
39	-
40	- def wins_plus_losses
41	- wins + losses
42	- end
43	-
44	- # TODO delete these and refactor loss_count and votes_count into losses and wins
45	- def losses
46	- loss_count \|\| 0
47	- end
48	-
49	- def wins
50	- votes_count \|\| 0
51	- end
52		24
53	def before_create	25	def before_create
54	unless self.score	26	unless self.score
@@ -0,0 +1,15 @@		@@ -0,0 +1,15 @@
	1	+class RemoveWinsLossesAndRenameCounts < ActiveRecord::Migration
	2	+ def self.up
	3	+ remove_column :choices, :wins
	4	+ remove_column :choices, :losses
	5	+ rename_column :choices, :votes_count, :wins
	6	+ rename_column :choices, :loss_count, :losses
	7	+ end
	8	+
	9	+ def self.down
	10	+ rename_column :choices, :wins, :votes_count
	11	+ rename_column :choices, :losses, :loss_count
	12	+ add_column :choices, :wins, :integer
	13	+ add_column :choices, :losses, :integer
	14	+ end
	15	+end
	@@ -173,7 +173,7 @@ describe Question do		@@ -173,7 +173,7 @@ describe Question do
173	it "should provide average voter information" do	173	it "should provide average voter information" do
174	params = {:id => 124, :visitor_identifier => "jim", :with_prompt => true, :with_appearance => true, :with_average_votes => true }	174	params = {:id => 124, :visitor_identifier => "jim", :with_prompt => true, :with_appearance => true, :with_average_votes => true }
175	@question_optional_information = @question.get_optional_information(params)	175	@question_optional_information = @question.get_optional_information(params)
176	- @question_optional_information[:average_votes].should be_an_instance_of(Float)	176	+ @question_optional_information[:average_votes].should be_an_instance_of(Fixnum)
177	@question_optional_information[:average_votes].should be_close(0.0, 0.1)	177	@question_optional_information[:average_votes].should be_close(0.0, 0.1)
178		178
179	vote_options = {:visitor_identifier => "jim",	179	vote_options = {:visitor_identifier => "jim",