From 39db2eaf49aee569e9d216d312abba7388e8529d Mon Sep 17 00:00:00 2001 From: John Dong Date: Fri, 17 Aug 2007 19:16:34 -0400 Subject: [PATCH 1/1] Improve consistency of XMLRPC error reporting. Now, all error conditions will raise an exception. Before, there was an inconsistent mix between raising exceptions and returning nil objects. Nil objects are much less desirable because the client must explicitly check for errors and some errors might silently pass through and produce incorrect results --- app/apis/selectricity_api.rb | 2 -- app/models/selectricity_service.rb | 16 ++++++---------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/app/apis/selectricity_api.rb b/app/apis/selectricity_api.rb index dfc9946..9c95369 100644 --- a/app/apis/selectricity_api.rb +++ b/app/apis/selectricity_api.rb @@ -19,12 +19,10 @@ class VoteResultStruct < ActionWebService::Struct member :condorcet_winners, [:int] member :ssd_winners, [:int] member :borda_winners, [:int] - member :errors, [:string] end class CandidateMap < ActionWebService::Struct member :candidate_ids, [:int] member :candidate_names, [:string] - member :errors, [:string] end class SelectricityAPI < ActionWebService::API::Base api_method :cast_quickvote, :expects => [:string, :int, [[:int]]], :returns => [:string] diff --git a/app/models/selectricity_service.rb b/app/models/selectricity_service.rb index add8154..196161a 100644 --- a/app/models/selectricity_service.rb +++ b/app/models/selectricity_service.rb @@ -28,7 +28,7 @@ class SelectricityService < ActionWebService::Base def quickvote_candidate_ids_to_names(shortname, id_list) qv=QuickVote.ident_to_quickvote(shortname) candidates={} - return [] unless qv + raise ArgumentError.new("Quickvote by name #{shortname} doesn't exist") unless qv qv.results qv.candidates.each {|c| candidates[c.id] = c} results=[] @@ -46,10 +46,8 @@ class SelectricityService < ActionWebService::Base #TODO: Validate shortname qv=QuickVote.ident_to_quickvote(shortname) result=VoteResultStruct.new - result.errors=[] unless qv - result.errors << "No quickvote with name #{shortname} found!" - return result + raise ArgumentError.new("No quickvote with name #{shortname} found!") end qv.results result.plurality_winners=qv.plurality_result.winners @@ -62,10 +60,8 @@ class SelectricityService < ActionWebService::Base def get_quickvote_candidate_map(shortname) qv=QuickVote.ident_to_quickvote(shortname) result=CandidateMap.new - result.errors=[] unless qv - result.errors << "No quickvote with name #{shortname} found!" - return result + raise ArgumentError.new("No quickvote with name #{shortname} found!") end candidates={} qv.candidates.each {|c| candidates[c.id] = c.name} @@ -77,7 +73,7 @@ class SelectricityService < ActionWebService::Base qv=QuickVote.ident_to_quickvote(shortname) votes=Array.new unless qv - return result + raise ArgumentError.new("Cannot find QuickVote #{shortname}") end qv.votes.each do |vote| votes << VoteInfo.new(:voter_id => vote.voter.id, :voter_ipaddress => vote.voter.ipaddress, :vote_time => vote.time.to_i, :vote => vote.votes, :voter_session_id => vote.voter.session_id ) @@ -92,7 +88,7 @@ class SelectricityService < ActionWebService::Base return all end def get_quickvote(shortname) - return ElectionStruct.new unless election=QuickVote.ident_to_quickvote(shortname) + raise ArgumentError.new("Cannot find QuickVote named #{shortname}") unless election=QuickVote.ident_to_quickvote(shortname) return ElectionStruct.new(:id => election.id, :name => election.name, :description => election.description, :candidate_ids => election.candidates.collect {|c| c.id }, :candidate_names => election.candidates.collect {|c| c.name } ) end def create_quickvote(election) @@ -101,7 +97,7 @@ class SelectricityService < ActionWebService::Base if qv.save return "" else - return "Saving quickvote FAILED:"+qv.errors.inspect + raise ArgumentError.new("Saving quickvote FAILED:"+qv.errors.inspect) end end -- 2.39.5