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
class VotingSystem #Hello, I am Rodger the old and unhappy programmer. the variable nbrOfVotes is an array of 2 dimensions. The first dimension contains the number of votes for the answer "YES, IT SUCKS"... and the other dimension contain the number of votes for the answer "NO, IT DOESN'T SUCK". In the near future there will be other possible answers... but I don't care! I retire in 2 days! @@nbrOfVotes = Array.new #The users who sent their vote arrive in this very top secret method!! (I retire in 2 days!) def receiveAVote(theVote) if theVote == "YES, IT SUCKS" @@nbrOfVotes[0] = 0 if @@nbrOfVotes[0].nil? @@nbrOfVotes[0] = @@nbrOfVotes[0] + 1 else if theVote == "NO, IT DOESN'T SUCK" @@nbrOfVotes[1] = 0 if @@nbrOfVotes[1].nil? @@nbrOfVotes[1] = @@nbrOfVotes[1] + 1 else puts "THIS IS NOT A VALID ANSWER YOU MORON... btw i retire in 2 days!" end end end #This is the function that compile ALL the votes... I retire in 2 days! def compileAllTheVotes for i in (0..1) if i == 0 @@nbrOfVotes[0] = 0 if @@nbrOfVotes[0].nil? puts "HERE IS THE NUMBER OF VOTES FOR 'YES IT SUCKS' : " + @@nbrOfVotes[0].to_s else if i == 1 @@nbrOfVotes[1] = 0 if @@nbrOfVotes[1].nil? puts "HERE IS THE NUMBER OF VOTES FOR 'NO IT DOESN'T SUCKS' : " + @@nbrOfVotes[1].to_s end end end end end
Refactorings
No refactoring yet !
Vince
April 17, 2008, April 17, 2008 00:37, permalink
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23
class VotingSystem def initialize @votes = { "yes, it sucks" => 0, "no, it doesn't suck" => 0, } end def vote(vote) vote.downcase! if @votes.has_key? vote @votes[vote] += 1 else puts "THIS IS NOT A VALID ANSWER YOU MORON… btw i retire in 2 days!" end end def compile_votes @votes.each do |vote, count| puts "HERE IS THE NUMBER OF VOTES FOR '#{vote}': #{count}" end end end
Sam Figueroa
April 17, 2008, April 17, 2008 12:39, permalink
almost the same as Vince. But he did a hell of a job, so there isn't much left to change.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21
class VotingSystem def initialize @votes = { "yes, it sucks" => 0, "no, it doesn't suck" => 0, "other" => 0} end def process_vote(vote) begin @votes[vote.downcase] += 1 rescue @votes['other'] += 1 end end def output total = @votes.values.inject(0) {|sum, n| sum + n } @votes.each do |answer, number| percent = number.to_f / total * 100.0 puts "Here is the number of votes for '#{answer}' : #{number} => #{percent}%" end end end
Vince
April 17, 2008, April 17, 2008 13:08, permalink
Cool I didn't know about the begin/rescue for the hash
FrankLamontagne
April 17, 2008, April 17, 2008 13:13, permalink
Here is my solution... nothing revolutionary but it does the job. I also added the possibility to vote on multiple questions
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16
class VotingSystem def vote(answer, question = "is 'the flinstones' a boring t.v. show?") if @@votes.has_key?(question.downcase) && @@votes[question.downcase].has_key?(answer.downcase) @@votes[question][answer.downcase] += 1 puts "There are #{@@votes[question.downcase][answer.downcase]-1} people who think like you!" else puts "Inexistant question or answer" end end private @@votes = { "is 'the flinstones' a boring t.v. show?" => {"yes, it sucks!" => 0, "no, it rocks!" => 0}, "what is your name?" => {"frank" => 0, "what's the point to vote on that?" => 0} } end
jswanner
April 17, 2008, April 17, 2008 13:53, permalink
not much different than what has already been submitted.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15
class VotingSystem @@votes = { "yes, it sucks" => 0, "no, it doesn't suck" => 0, "other" => 0} def cast_vote(vote) @@votes[vote.downcase] += 1 rescue @@votes['other'] += 1 end def results @@votes.each do |vote, count| puts "there are #{count} vote(s) for '#{vote}'" end end end
fifoo
April 18, 2008, April 18, 2008 22:00, permalink
Maybe not quite a refactoring...
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 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83
# A choice in a Voting System class VotingChoice attr_reader :label def initialize(label = "Empty Choice") @label = label end def to_s @label end end # THE Voting System class VotingSystem # Constructor # [choices] an array of available choices to the user (you know what they are on opening the vote and they cannot change!) def initialize(choices) choices << VotingChoice.new("Abstention") # If the user does not know what to vote for # A hash of the available choices, used for the UI @choices = choices.collect { |c| c.label.to_sym } # A hash to record the votes @votes = Hash.new @choices.each_index do |idx| @votes[@choices[idx]] = 0 end # Let's not forget the invalid ones @votes[:invalid] = 0 end # Display the choices and record user's vote def vote @choices.each do |choice| puts " " + choice_id(choice).to_s + ". " + choice.to_s end print "Please vote: " if choice = STDIN.gets record_vote(choice.to_i) end end # Display the results def results puts "\nResults of the vote:" # Go through the choices rather than votes so we get a consistent order @choices.each do |choice| puts " " + choice.to_s + ": " + @votes[choice].to_s end # And the invalids puts " Invalid: " + @votes[:invalid].to_s end private def record_vote(choice_id) if valid_choice?(choice_id) @votes[@choices.at(choice_id-1)] += 1 else puts "THIS IS NOT A VALID ANSWER YOU #*%!... I could do that for an extra 2 years!" @votes[:invalid] += 1 end end # Return a unique id for the choice. We could have used the array index directly # but starting at 0 is not user friendly and dangerous ( "string".to_i => 0 )! def choice_id(choice) @choices.index(choice) + 1 end def valid_choice?(choice_id) ( choice_id > 0 ) && !@choices.at(choice_id-1).nil? end end vc = [ VotingChoice.new("YES, IT SUCKS"), VotingChoice.new("NO, IT DOESN'T SUCK"), VotingChoice.new("THERE MUST BE A BETTER WAY") ] vs = VotingSystem.new(vc) 3.times { vs.vote } vs.results
we4tech
April 19, 2008, April 19, 2008 07:29, permalink
most of the suggestions are great, here is my one.
while i complied this suggestion i had the following points in mind -
1. extensibility
2. simplicity in terms of exposing api
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 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90
# voting system related code class Topic attr_accessor :name, :answers def initialize(p_topic_name) @name = p_topic_name @answers = {} end def add_answer(p_answer) @answers[p_answer.downcase] = 0 return self end def select_answer(p_answer) answer = p_answer.downcase verify_valid_answer(answer) @answers[answer] = @answers[answer] + 1 end def count_response(p_answer) answer = p_answer.downcase verify_valid_answer(answer) return @answers[answer] end private def verify_valid_answer(p_answer) raise "You have selected an unknown answer - #{p_answer}" if !@answers.has_key?(p_answer) end end class VotingSystem @@topics = {} def self.add_topic(p_topic) @@topics[p_topic.name.downcase] = p_topic return self end def self.cast(p_topic_name, p_answer) topic_name = p_topic_name.downcase verify_valid_topic(topic_name) @@topics[topic_name].select_answer(p_answer) end # display all results if p_topic_name is not specified def self.result(p_topic_name = nil) if p_topic_name.nil? @@topics.each do |e_topic_name, e_topic| show_result_from(e_topic) end else topic_name = p_topic_name.downcase verify_valid_topic(topic_name) show_result_from(@@topics[topic_name]) end end private def self.verify_valid_topic(p_topic_name) raise "You have selected an unknown topic - #{p_topic_name}" if !@@topics.has_key?(p_topic_name) end def self.show_result_from(p_topic) puts "Topic - '#{p_topic.name}'" p_topic.answers.each do |e_answer, e_count| puts "#{e_count} people agreed on '#{e_answer}'" end end end # usages # setup voting system with topics VotingSystem.add_topic(Topic.new("is 'the flinstones' a boring t.v. show?").add_answer("yes, it sucks!").add_answer("no, it rocks!")) # yet another topic to vote VotingSystem.add_topic(Topic.new("should i ask again?").add_answer("yes, go on!").add_answer("no, its ok!")) # add few casts VotingSystem.cast("is 'the flinstones' a boring t.v. show?", "yes, it sucks!") VotingSystem.cast("should i ask again?", "no, its ok!") # view all results VotingSystem.result # view specific result puts "--------------" VotingSystem.result("is 'the flinstones' a boring t.v. show?")
danielharan
May 4, 2008, May 04, 2008 00:11, permalink
System is a vile word in a class name. It is meaningless and encourages a pile up of concerns that should remain separated.
Since that dude retired 2 weeks ago, I think it's safe to scrap that piece of code. Normally it's better to refactor than re-write... but this is egregious.
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
class Poll attr_accessor :question def initialize(question, answers=[]) @question, @answers = question, {} answers.each {|key| @answers[key] = 0} end def vote_for(answer) raise 'unknown answer' unless @answers.has_key?(answer) @answers[answer] += 1 end def results @answers.each { |answer, votes| puts "#{votes} : #{answer}" } puts "Out of #{@answers.values.inject {|memo,v| v + memo}} votes" end end >> p = Poll.new("can haz chezburger?", ["laterz", "nao"]) => #<Poll:0x35cb2c @answers={"laterz"=>0, "nao"=>0}, @question="can haz chezburger?"> >> p.vote_for "nao" => 1 >> p.vote_for "nao" => 2 >> p.vote_for "laterz" => 1 >> p.results 1 : laterz 2 : nao Out of 3 votes
Ouch... this guy badly needs (but doesn't deserve) help! Original article on RubyFleebie is here : http://www.rubyfleebie.com/rubyize-this-6th-edition