From 373e66542820382f7590cfd81f60e6699890c05e Mon Sep 17 00:00:00 2001 From: Katsuya Noguchi Date: Sun, 15 Jul 2012 01:51:11 -0700 Subject: [PATCH] Refactor codes by removing duplication --- lib/reputation_system.rb | 3 +- lib/reputation_system/.normalization.rb.swp | Bin 0 -> 12288 bytes lib/reputation_system/.reputation.rb.swp | Bin 0 -> 12288 bytes lib/reputation_system/base.rb | 1 - lib/reputation_system/evaluation.rb | 89 +++++++++---------- lib/reputation_system/network.rb | 1 + lib/reputation_system/normalization.rb | 50 ----------- lib/reputation_system/reputation.rb | 41 +++++++-- spec/reputation_system/normalization_spec.rb | 42 +-------- spec/reputation_system/reputation_spec.rb | 43 +++++++++ 10 files changed, 120 insertions(+), 150 deletions(-) create mode 100644 lib/reputation_system/.normalization.rb.swp create mode 100644 lib/reputation_system/.reputation.rb.swp delete mode 100644 lib/reputation_system/normalization.rb diff --git a/lib/reputation_system.rb b/lib/reputation_system.rb index 7c7d333..0356dc6 100644 --- a/lib/reputation_system.rb +++ b/lib/reputation_system.rb @@ -16,7 +16,6 @@ require 'reputation_system/base' require 'reputation_system/query' -require 'reputation_system/normalization' require 'reputation_system/evaluation' require 'reputation_system/network' require 'reputation_system/reputation' @@ -25,4 +24,4 @@ require 'models/rs_reputation' require 'models/rs_reputation_message' -ActiveRecord::Base.send(:include, ReputationSystem::Base) \ No newline at end of file +ActiveRecord::Base.send(:include, ReputationSystem::Base) diff --git a/lib/reputation_system/.normalization.rb.swp b/lib/reputation_system/.normalization.rb.swp new file mode 100644 index 0000000000000000000000000000000000000000..dad8b4ca6c2024411e4b6dc8d8aa91658f46e454 GIT binary patch literal 12288 zcmeI2&u<$=6o4ljQ2Eh<0C9demw@bbnjQe7fZQaARa+IFm(T#*p}0rv_Ci8tQ0ybW;^wW0`RR{FBLvu|hKd^5Hb z`3+Vt`&YU?w?ybyK1MuTzdhYeWrgB zO}IHb1I~am;0!ne&VV!E3^)TP)_}>D$a^^ZM@j?FmFJ}c=knH_oB?OR8E^)i0cXG& za0Z+KXTTY72AlzB;D2a<;_v)tP7`t+vA_S%{{R2$DMEfnenV~|edHx%1^M#~A$O3k zkpuzcSA73F&^$!;^%3f_#j8fV_*ONDukt2|~U?zC^x2K0`i2 z6!OR8gxp4cMs6YBBcCF#B0oMx$PTiFJc`^tg*hM^QHVR70cXG&a0Z+KXW&0Kz(hm< ziof$SfPyzbc}3|Tv|vaR%`ng&3f7?;bc;g3h3_wXaCFBHmQrQl$|dGq+!Zr8-sG^# z(Hn9Rt+a+*X;X(*C`H6=^`nOM*WsO3rSMVEl-WjXdg$Z+n*$!F?|IDZip?(U_Bz!4 zkpWnAdas>#zl&3rukP9P@)jj=uzh|%yE=M;nE!+AWV_GqYyVZJxG;x<59ar?t7A>E zxHW+)LQauHSoWyWfM|cDLK^^!%*=w!5&g-P!be{&olN-h_7N3S96zn{{9u z(@?;!Wr}GHI3!QAgtKUxQpB}Ue2^O!{WEFc8ma+O02NSc2jB-aINuKZpl%cDi^O)V z*xNZ**wJ7DG{Z4MIzWjD9otc9tQbBs61ae|!f#L!*Fnpn8B@i`RN=^&tkrCe$77FT z-T8=lQpH$ZK2@lj7yXT`PO#Nj^Hz)aD>8?aPCyO}#a=0dGQ+#!U6!e_IZnz@Ar0i( zt^-bP$T2^5Eo7OoS%uP?p(123Mr+kxrn1VlteR?1)10kx9WFCv*9tI2Sh4G0D_^R; e_*#+k2BxTZJTkDhdUg$ZIFezi4!#KOM*16kva<#N literal 0 HcmV?d00001 diff --git a/lib/reputation_system/.reputation.rb.swp b/lib/reputation_system/.reputation.rb.swp new file mode 100644 index 0000000000000000000000000000000000000000..a3c0d6daf4c450a8b2aa74059faa5299c8f22157 GIT binary patch literal 12288 zcmeI2Ux*t;9LJ}nYOPkQg5cY)xtF_|Y_8XbN~A5<_QcROS1xy!QY1`vC&_x7-8eIQ zX>4m1g@OozPl^wMK1rX%N_|r(LKPoG6cnn6h|fM&rQmlq$z_w=X?tKnI&*xI-PxJn zpYP1h+;LN#ex`Vg_BwkQ$_~bwFKs^l+LbT1KI$>H%(aR;JdEnG+we_;wabxiyIiT; zQ00zs_h1w)=Z?$B2_lyV-F`0)8_XXTD#k zDO$1S+KL@Lm`M|H{2ep-6?V!ey)wLtFwN`;URDoF6a!+h$duDb|8wDmZJL!>! zm+nw8Y}!@6|f3e1*`)9Lj^eg&wuqU#;)Cs;PLo>qE82rkz-90OXoD@_t$P^T z1Aa%|zk#2D(cnvP3A_uQ1Ajo9m%$3y3vO&->{swTxB@PKtH2hkfK_0_3W(5SMBPt9 z8C(rt6D^b%R?h=DH9zz!Lw!Zq{}~t@YLGU*?gKO$_Y=vAD+Sl0BE`mgCTE#a-BXt) z@uAHBIfnkJs7WQmEGZGxvec51E0mgIi7kBeDrg@KWGkCfRJYylh8&J{5~i#Oy0-^hc`ViQ1$YD8~)BQclR50wX8GsZ#}BuS-$K?D8tr z&a@}e6;CMkeJL`xR40HbD(s`*IZ4} z4=nrxMM;4=|Cn+%}yn`?JJ&2#Es%mSK7XAr1DuO zzAP1Z5M){6f7K{PF0nUAuj{M=QPwfkmnIn+^yjj!u*^xN6Wc~H$;0KfE$lL_r$)t* zbbzGOJyJ;ps_{UHUZB#+#5=?#HGi?$L zd~Z;#JFTR1N^E2ATsQe7$JKX6Rg%cfi?PN$O7a-YgTWOcWyM#-&R(YGz9+%HAqmz< z-;CsV@*7GCcKTlTla?`ZbOd)B(mmm=_{42ogDit%$z7LK0Uv*NGE0&7G_I}#kn%gFVLa+(&6G_ zalVARr)aixl8zKhhqENGULqE8x+U?82*IV_Yz4mXl8`)KY3WyEyv^f5n|&qSAPR|x zl$kA4v7DiUv*ltrYXVvd17li>=VM>OP_;w61s^W2LcxHyjaQEC6wh=-en@SJ?I5gU zGl^=t&81+8gbl4*g?zr&~jb&P? z*(A*VC}-&zADL>YW~O(~&>}o(fqlIgx@IE%4Rrf9 A9RL6T literal 0 HcmV?d00001 diff --git a/lib/reputation_system/base.rb b/lib/reputation_system/base.rb index d9087e5..c9a7249 100644 --- a/lib/reputation_system/base.rb +++ b/lib/reputation_system/base.rb @@ -51,7 +51,6 @@ def has_reputation(reputation_name, options) unless ancestors.include?(ReputationSystem::Reputation) has_many :reputations, :as => :target, :class_name => "RSReputation", :dependent => :destroy include ReputationSystem::Query - include ReputationSystem::Normalization include ReputationSystem::Reputation include ReputationSystem::Scope end diff --git a/lib/reputation_system/evaluation.rb b/lib/reputation_system/evaluation.rb index 6e46500..a71235a 100644 --- a/lib/reputation_system/evaluation.rb +++ b/lib/reputation_system/evaluation.rb @@ -17,7 +17,6 @@ module ReputationSystem module Evaluation def add_evaluation(reputation_name, value, source, *args) - raise ArgumentError, "#{reputation_name.to_s} is not defined for #{self.class.name}" unless ReputationSystem::Network.has_reputation_for?(self.class.name, reputation_name) scope = args.first srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) process = ReputationSystem::Network.get_reputation_def(self.class.name, srn)[:aggregated_by] @@ -27,63 +26,32 @@ def add_evaluation(reputation_name, value, source, *args) end def update_evaluation(reputation_name, value, source, *args) - raise ArgumentError, "#{reputation_name.to_s} is not defined for #{self.class.name}" unless ReputationSystem::Network.has_reputation_for?(self.class.name, reputation_name) - scope = args.first - srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) - evaluation = RSEvaluation.find_by_reputation_name_and_source_and_target(srn, source, self) - if evaluation.nil? - raise ArgumentError, "Given instance of #{source.class.name} has not evaluated #{reputation_name} of the instance of #{self.class.name} yet." - else - oldValue = evaluation.value - evaluation.value = value - evaluation.save! - process = ReputationSystem::Network.get_reputation_def(self.class.name, srn)[:aggregated_by] - rep = RSReputation.find_by_reputation_name_and_target(srn, self) - RSReputation.update_reputation_value_with_updated_source(rep, evaluation, oldValue, 1, process) - end + srn, evaluation = find_srn_and_evaluation!(reputation_name, source, args.first) + oldValue = evaluation.value + evaluation.value = value + evaluation.save! + process = ReputationSystem::Network.get_reputation_def(self.class.name, srn)[:aggregated_by] + rep = RSReputation.find_by_reputation_name_and_target(srn, self) + RSReputation.update_reputation_value_with_updated_source(rep, evaluation, oldValue, 1, process) end def add_or_update_evaluation(reputation_name, value, source, *args) - scope = args.first - srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) - evaluation = RSEvaluation.find_by_reputation_name_and_source_and_target(srn, source, self) - if evaluation.nil? - self.add_evaluation(reputation_name, value, source, scope) + srn, evaluation = find_srn_and_evaluation(reputation_name, source, args.first) + if RSEvaluation.exists? :reputation_name => srn, :source_id => source.id, :source_type => source.class.name, :target_id => self.id, :target_type => self.class.name + self.update_evaluation(reputation_name, value, source, *args) else - self.update_evaluation(reputation_name, value, source, scope) + self.add_evaluation(reputation_name, value, source, *args) end end def delete_evaluation(reputation_name, source, *args) - raise ArgumentError, "#{reputation_name.to_s} is not defined for #{self.class.name}" unless ReputationSystem::Network.has_reputation_for?(self.class.name, reputation_name) - scope = args.first - srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) - evaluation = RSEvaluation.find_by_reputation_name_and_source_and_target(srn, source, self) - unless evaluation.nil? - process = ReputationSystem::Network.get_reputation_def(self.class.name, srn)[:aggregated_by] - oldValue = evaluation.value - evaluation.value = process == :product ? 1 : 0 - rep = RSReputation.find_by_reputation_name_and_target(srn, self) - RSReputation.update_reputation_value_with_updated_source(rep, evaluation, oldValue, 1, process) - evaluation.destroy - end + srn, evaluation = find_srn_and_evaluation(reputation_name, source, args.first) + delete_evaluation_without_validation(srn, evaluation) if evaluation end def delete_evaluation!(reputation_name, source, *args) - raise ArgumentError, "#{reputation_name.to_s} is not defined for #{self.class.name}" unless ReputationSystem::Network.has_reputation_for?(self.class.name, reputation_name) - scope = args.first - srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) - evaluation = RSEvaluation.find_by_reputation_name_and_source_and_target(srn, source, self) - if evaluation.nil? - raise ArgumentError, "Given instance of #{source.class.name} has not evaluated #{reputation_name} of the instance of #{self.class.name} yet." - else - process = ReputationSystem::Network.get_reputation_def(self.class.name, srn)[:aggregated_by] - oldValue = evaluation.value - evaluation.value = process == :product ? 1 : 0 - rep = RSReputation.find_by_reputation_name_and_target(srn, self) - RSReputation.update_reputation_value_with_updated_source(rep, evaluation, oldValue, 1, process) - evaluation.destroy - end + srn, evaluation = find_srn_and_evaluation!(reputation_name, source, args.first) + delete_evaluation_without_validation(srn, evaluation) end def increase_evaluation(reputation_name, value, source, *args) @@ -95,6 +63,33 @@ def decrease_evaluation(reputation_name, value, source, *args) end protected + def find_srn_and_evaluation(reputation_name, source, scope) + srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) + evaluation = RSEvaluation.find_by_reputation_name_and_source_and_target(srn, source, self) + return srn, evaluation + end + + def find_srn_and_evaluation!(reputation_name, source, scope) + srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) + evaluation = find_evaluation!(reputation_name, srn, source) + return srn, evaluation + end + + def find_evaluation!(reputation_name, srn, source) + evaluation = RSEvaluation.find_by_reputation_name_and_source_and_target(srn, source, self) + raise ArgumentError, "Given instance of #{source.class.name} has not evaluated #{reputation_name} of the instance of #{self.class.name} yet." unless evaluation + evaluation + end + + def delete_evaluation_without_validation(srn, evaluation) + process = ReputationSystem::Network.get_reputation_def(self.class.name, srn)[:aggregated_by] + oldValue = evaluation.value + evaluation.value = process == :product ? 1 : 0 + rep = RSReputation.find_by_reputation_name_and_target(srn, self) + RSReputation.update_reputation_value_with_updated_source(rep, evaluation, oldValue, 1, process) + evaluation.destroy + end + def change_evaluation_value_by(reputation_name, value, source, *args) scope = args.first srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) diff --git a/lib/reputation_system/network.rb b/lib/reputation_system/network.rb index 0c925a1..88ffb99 100644 --- a/lib/reputation_system/network.rb +++ b/lib/reputation_system/network.rb @@ -82,6 +82,7 @@ def has_scope?(class_name, reputation_name, scope) end def get_scoped_reputation_name(class_name, reputation_name, scope) + raise ArgumentError, "#{reputation_name.to_s} is not defined for #{class_name}" unless has_reputation_for?(class_name, reputation_name) scope = scope.to_sym if scope validate_scope_necessity(class_name, reputation_name, scope) validate_scope_existence(class_name, reputation_name, scope) diff --git a/lib/reputation_system/normalization.rb b/lib/reputation_system/normalization.rb deleted file mode 100644 index 2d4155e..0000000 --- a/lib/reputation_system/normalization.rb +++ /dev/null @@ -1,50 +0,0 @@ -## -# Copyright 2012 Twitter, Inc -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -## - -module ReputationSystem - module Normalization - def normalized_reputation_value_for(reputation_name, *args) - scope = args.first - if !self.class.has_reputation_for?(reputation_name) - raise ArgumentError, "#{reputation_name} is not valid" - else - reputation_name = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) - process = ReputationSystem::Network.get_reputation_def(self.class.name, reputation_name)[:aggregated_by] - reputation = RSReputation.find_or_create_reputation(reputation_name, self, process) - reputation.normalized_value - end - end - - def activate_all_reputations - RSReputation.find(:all, :conditions => {:target_id => self.id, :target_type => self.class.name, :active => false}).each do |r| - r.active = true - r.save! - end - end - - def deactivate_all_reputations - RSReputation.find(:all, :conditions => {:target_id => self.id, :target_type => self.class.name, :active => true}).each do |r| - r.active = false - r.save! - end - end - - def reputations_activated?(reputation_name) - r = RSReputation.find(:first, :conditions => {:reputation_name => reputation_name.to_s, :target_id => self.id, :target_type => self.class.name}) - r ? r.active : false - end - end -end \ No newline at end of file diff --git a/lib/reputation_system/reputation.rb b/lib/reputation_system/reputation.rb index 51035fa..2441af3 100644 --- a/lib/reputation_system/reputation.rb +++ b/lib/reputation_system/reputation.rb @@ -17,17 +17,32 @@ module ReputationSystem module Reputation def reputation_value_for(reputation_name, *args) - scope = args.first - if !self.class.has_reputation_for?(reputation_name) - raise ArgumentError, "#{reputation_name} is not valid" - else - reputation_name = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) - process = ReputationSystem::Network.get_reputation_def(self.class.name, reputation_name)[:aggregated_by] - reputation = RSReputation.find_or_create_reputation(reputation_name, self, process) - reputation.value + find_reputation(reputation_name, args.first).value + end + + def normalized_reputation_value_for(reputation_name, *args) + find_reputation(reputation_name, args.first).normalized_value + end + + def activate_all_reputations + RSReputation.find(:all, :conditions => {:target_id => self.id, :target_type => self.class.name, :active => false}).each do |r| + r.active = true + r.save! + end + end + + def deactivate_all_reputations + RSReputation.find(:all, :conditions => {:target_id => self.id, :target_type => self.class.name, :active => true}).each do |r| + r.active = false + r.save! end end + def reputations_activated?(reputation_name) + r = RSReputation.find(:first, :conditions => {:reputation_name => reputation_name.to_s, :target_id => self.id, :target_type => self.class.name}) + r ? r.active : false + end + def rank_for(reputation_name, *args) scope = args.first my_value = self.reputation_value_for(reputation_name, scope) @@ -35,5 +50,13 @@ def rank_for(reputation_name, *args) :conditions => ["rs_reputations.value > ?", my_value] ) + 1 end + + protected + def find_reputation(reputation_name, scope) + raise ArgumentError, "#{reputation_name} is not valid" if !self.class.has_reputation_for?(reputation_name) + srn = ReputationSystem::Network.get_scoped_reputation_name(self.class.name, reputation_name, scope) + process = ReputationSystem::Network.get_reputation_def(self.class.name, srn)[:aggregated_by] + RSReputation.find_or_create_reputation(srn, self, process) + end end -end \ No newline at end of file +end diff --git a/spec/reputation_system/normalization_spec.rb b/spec/reputation_system/normalization_spec.rb index f06bed4..6ce0d4e 100644 --- a/spec/reputation_system/normalization_spec.rb +++ b/spec/reputation_system/normalization_spec.rb @@ -25,44 +25,4 @@ @phrase = Phrase.create!(:text => "One") end - describe "#normalized_reputation_value_for" do - it "should return 0 as if there is no data" do - @question.normalized_reputation_value_for(:total_votes).should == 0 - end - - it "should return appropriate value in case of valid input" do - question2 = Question.create!(:text => 'Does this work too?', :author_id => @user.id) - question3 = Question.create!(:text => 'Does this work too?', :author_id => @user.id) - @question.add_evaluation(:total_votes, 1, @user) - question2.add_evaluation(:total_votes, 2, @user) - question3.add_evaluation(:total_votes, 3, @user) - @question.normalized_reputation_value_for(:total_votes).should == 0 - question2.normalized_reputation_value_for(:total_votes).should == 0.5 - question3.normalized_reputation_value_for(:total_votes).should == 1 - end - - it "should raise exception if invalid reputation name is given" do - lambda {@question.normalized_reputation_value_for(:invalid)}.should raise_error(ArgumentError) - end - - it "should raise exception if scope is given for reputation with no scopes" do - lambda {@question.normalized_reputation_value_for(:difficulty, :s1)}.should raise_error(ArgumentError) - end - - it "should raise exception if scope is not given for reputation with scopes" do - lambda {@phrase.normalized_reputation_value_for(:difficulty_with_scope)}.should raise_error(ArgumentError) - end - end - - describe "#exclude_all_reputations_for_normalization" do - it "should activate all reputation" do - @question2 = Question.create!(:text => 'Does this work??', :author_id => @user.id) - @question2.add_evaluation(:total_votes, 70, @user) - @question.add_evaluation(:total_votes, 100, @user) - @question.deactivate_all_reputations - RSReputation.maximum(:value, :conditions => {:reputation_name => 'total_votes', :active => true}).should == 70 - @question.activate_all_reputations - RSReputation.maximum(:value, :conditions => {:reputation_name => 'total_votes', :active => true}).should == 100 - end - end -end \ No newline at end of file +end diff --git a/spec/reputation_system/reputation_spec.rb b/spec/reputation_system/reputation_spec.rb index 739cfca..0e897d2 100644 --- a/spec/reputation_system/reputation_spec.rb +++ b/spec/reputation_system/reputation_spec.rb @@ -116,4 +116,47 @@ end end end + + context "Normalization" do + describe "#normalized_reputation_value_for" do + it "should return 0 as if there is no data" do + @question.normalized_reputation_value_for(:total_votes).should == 0 + end + + it "should return appropriate value in case of valid input" do + question2 = Question.create!(:text => 'Does this work too?', :author_id => @user.id) + question3 = Question.create!(:text => 'Does this work too?', :author_id => @user.id) + @question.add_evaluation(:total_votes, 1, @user) + question2.add_evaluation(:total_votes, 2, @user) + question3.add_evaluation(:total_votes, 3, @user) + @question.normalized_reputation_value_for(:total_votes).should == 0 + question2.normalized_reputation_value_for(:total_votes).should == 0.5 + question3.normalized_reputation_value_for(:total_votes).should == 1 + end + + it "should raise exception if invalid reputation name is given" do + lambda {@question.normalized_reputation_value_for(:invalid)}.should raise_error(ArgumentError) + end + + it "should raise exception if scope is given for reputation with no scopes" do + lambda {@question.normalized_reputation_value_for(:difficulty, :s1)}.should raise_error(ArgumentError) + end + + it "should raise exception if scope is not given for reputation with scopes" do + lambda {@phrase.normalized_reputation_value_for(:difficulty_with_scope)}.should raise_error(ArgumentError) + end + end + + describe "#exclude_all_reputations_for_normalization" do + it "should activate all reputation" do + @question2 = Question.create!(:text => 'Does this work??', :author_id => @user.id) + @question2.add_evaluation(:total_votes, 70, @user) + @question.add_evaluation(:total_votes, 100, @user) + @question.deactivate_all_reputations + RSReputation.maximum(:value, :conditions => {:reputation_name => 'total_votes', :active => true}).should == 70 + @question.activate_all_reputations + RSReputation.maximum(:value, :conditions => {:reputation_name => 'total_votes', :active => true}).should == 100 + end + end + end end