From 0d9196a951401071e64597fb5f201e0d0ef35d5f Mon Sep 17 00:00:00 2001 From: RunasSudo Date: Tue, 29 Jun 2021 15:31:38 +1000 Subject: [PATCH] Refactoring --- src/constraints.rs | 4 +- src/election.rs | 12 ---- src/main.rs | 40 +++++------- src/numbers/fixed.rs | 6 -- src/numbers/gfixed.rs | 6 -- src/numbers/mod.rs | 2 +- src/numbers/native.rs | 4 -- src/numbers/rational_num.rs | 8 --- src/numbers/rational_rug.rs | 7 --- src/stv/gregory.rs | 69 ++++++++++++--------- src/stv/meek.rs | 24 ++++---- src/stv/mod.rs | 118 +++++++++++++----------------------- src/stv/wasm.rs | 6 +- src/ties.rs | 82 +++++++++++++++++++++---- tests/utils/mod.rs | 2 +- 15 files changed, 187 insertions(+), 203 deletions(-) diff --git a/src/constraints.rs b/src/constraints.rs index d931c78..f691b04 100644 --- a/src/constraints.rs +++ b/src/constraints.rs @@ -227,7 +227,7 @@ impl ConstraintMatrix { }).collect(); let cell = &mut self[&idx[..]]; - let count_card = candidates.get(candidate).unwrap(); + let count_card = &candidates[candidate]; match count_card.state { CandidateState::Hopeful | CandidateState::Guarded => { cell.cands += 1; @@ -468,7 +468,7 @@ impl ops::IndexMut<&[usize]> for ConstraintMatrix { fn candidates_in_constraint_cell<'a, N: Number>(election: &'a Election, candidates: &HashMap<&Candidate, CountCard>, idx: &[usize]) -> Vec<&'a Candidate> { let mut result: Vec<&Candidate> = Vec::new(); for (i, candidate) in election.candidates.iter().enumerate() { - let cc = candidates.get(candidate).unwrap(); + let cc = &candidates[candidate]; if cc.state != CandidateState::Hopeful { continue; } diff --git a/src/election.rs b/src/election.rs index 24bb3f1..a39cf96 100644 --- a/src/election.rs +++ b/src/election.rs @@ -257,18 +257,6 @@ impl<'a, N: Number> CountState<'a, N> { } } -/// Result of a stage of counting -pub struct StageResult<'a, N: Number> { - /// See [CountState::kind] - pub kind: Option<&'a str>, - /// See [CountState::title] - pub title: &'a String, - /// Detailed logs of this stage, rendered from [CountState::logger] - pub logs: Vec, - /// Reference to the [CountState] of this stage - pub state: &'a CountState<'a, N>, -} - /// Current state of a [Candidate] during an election count #[derive(Clone)] pub struct CountCard<'a, N> { diff --git a/src/main.rs b/src/main.rs index a5ddc6d..8757afc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -16,7 +16,7 @@ */ use opentally::constraints::Constraints; -use opentally::election::{Candidate, CandidateState, CountCard, CountState, Election, StageResult}; +use opentally::election::{Candidate, CandidateState, CountCard, CountState, Election}; use opentally::numbers::{Fixed, GuardedFixed, NativeFloat64, Number, Rational}; use opentally::stv; @@ -286,7 +286,7 @@ where // Distribute first preferences stv::count_init(&mut state, &stv_opts).unwrap(); let mut stage_num = 1; - make_and_print_result(stage_num, &state, &cmd_opts); + print_stage(stage_num, &state, &cmd_opts); loop { let is_done = stv::count_one_stage(&mut state, &stv_opts); @@ -294,7 +294,7 @@ where break; } stage_num += 1; - make_and_print_result(stage_num, &state, &cmd_opts); + print_stage(stage_num, &state, &cmd_opts); } println!("Count complete. The winning candidates are, in order of election:"); @@ -305,7 +305,7 @@ where winners.push((candidate, count_card)); } } - winners.sort_unstable_by(|a, b| a.1.order_elected.partial_cmp(&b.1.order_elected).unwrap()); + winners.sort_unstable_by(|a, b| a.1.order_elected.cmp(&b.1.order_elected)); for (i, (winner, count_card)) in winners.into_iter().enumerate() { if let Some(kv) = &count_card.keep_value { @@ -350,15 +350,15 @@ fn print_candidates<'a, N: 'a + Number, I: Iterator(stage_num: usize, result: &StageResult, cmd_opts: &STV) { - // Print stage details - match result.kind { - None => { println!("{}. {}", stage_num, result.title); } - Some(kind) => { println!("{}. {} {}", stage_num, kind, result.title); } - }; - println!("{}", result.logs.join(" ")); +fn print_stage(stage_num: usize, state: &CountState, cmd_opts: &STV) { + let logs = state.logger.render(); - let state = result.state; + // Print stage details + match state.kind { + None => { println!("{}. {}", stage_num, state.title); } + Some(kind) => { println!("{}. {} {}", stage_num, kind, state.title); } + }; + println!("{}", logs.join(" ")); // Print candidates if cmd_opts.sort_votes { @@ -366,13 +366,13 @@ fn print_stage(stage_num: usize, result: &StageResult, cmd_opts: & let mut candidates: Vec<(&Candidate, &CountCard)> = state.candidates.iter() .map(|(c, cc)| (*c, cc)).collect(); // First sort by order of election (as a tie-breaker, if votes are equal) - candidates.sort_unstable_by(|a, b| b.1.order_elected.partial_cmp(&a.1.order_elected).unwrap()); + candidates.sort_unstable_by(|a, b| b.1.order_elected.cmp(&a.1.order_elected)); // Then sort by votes - candidates.sort_by(|a, b| a.1.votes.partial_cmp(&b.1.votes).unwrap()); + candidates.sort_by(|a, b| a.1.votes.cmp(&b.1.votes)); print_candidates(candidates.into_iter().rev(), cmd_opts); } else { let candidates = state.election.candidates.iter() - .map(|c| (c, state.candidates.get(c).unwrap())); + .map(|c| (c, &state.candidates[c])); print_candidates(candidates, cmd_opts); } @@ -392,13 +392,3 @@ fn print_stage(stage_num: usize, result: &StageResult, cmd_opts: & println!(""); } - -fn make_and_print_result(stage_num: usize, state: &CountState, cmd_opts: &STV) { - let result = StageResult { - kind: state.kind, - title: &state.title, - logs: state.logger.render(), - state: &state, - }; - print_stage(stage_num, &result, &cmd_opts); -} diff --git a/src/numbers/fixed.rs b/src/numbers/fixed.rs index 8094129..f9d696c 100644 --- a/src/numbers/fixed.rs +++ b/src/numbers/fixed.rs @@ -126,12 +126,6 @@ impl From for Fixed { fn from(n: usize) -> Self { Self(IBig::from(n) * get_factor()) } } -impl From for Fixed { - fn from(n: f64) -> Self { - return Self(IBig::from((n * 10_f64.powi(get_dps() as i32)).round() as u32)) - } -} - // TODO: Fix rounding impl fmt::Display for Fixed { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { diff --git a/src/numbers/gfixed.rs b/src/numbers/gfixed.rs index ee108a7..ef849a1 100644 --- a/src/numbers/gfixed.rs +++ b/src/numbers/gfixed.rs @@ -159,12 +159,6 @@ impl From for GuardedFixed { fn from(n: usize) -> Self { Self(IBig::from(n) * get_factor()) } } -impl From for GuardedFixed { - fn from(n: f64) -> Self { - return Self(IBig::from((n * 10_f64.powi((get_dps() * 2) as i32)).round() as u32)) - } -} - impl fmt::Display for GuardedFixed { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let dps = match f.precision() { diff --git a/src/numbers/mod.rs b/src/numbers/mod.rs index 37d317e..1a67d78 100644 --- a/src/numbers/mod.rs +++ b/src/numbers/mod.rs @@ -45,7 +45,7 @@ pub trait Assign { /// Trait for OpenTally numeric representations //pub trait Number: NumRef + NumAssignRef + PartialOrd + Assign + Clone + fmt::Display where for<'a> &'a Self: RefNum<&'a Self> { pub trait Number: - NumRef + NumAssignRef + ops::Neg + Ord + Assign + From + From + Clone + fmt::Debug + fmt::Display + NumRef + NumAssignRef + ops::Neg + Ord + Assign + From + Clone + fmt::Debug + fmt::Display where for<'a> Self: Assign<&'a Self> { diff --git a/src/numbers/native.rs b/src/numbers/native.rs index 3278f6f..5f85bf1 100644 --- a/src/numbers/native.rs +++ b/src/numbers/native.rs @@ -71,10 +71,6 @@ impl From for NativeFloat64 { fn from(n: usize) -> Self { Self(n as ImplType) } } -impl From for NativeFloat64 { - fn from(n: f64) -> Self { Self(n as ImplType) } -} - impl One for NativeFloat64 { fn one() -> Self { Self(1.0) } } diff --git a/src/numbers/rational_num.rs b/src/numbers/rational_num.rs index cf17263..852472f 100644 --- a/src/numbers/rational_num.rs +++ b/src/numbers/rational_num.rs @@ -108,14 +108,6 @@ impl From for Rational { fn from(n: usize) -> Self { Self(RatioType::from_integer(RatioBase::from(n))) } } -impl From for Rational { - fn from(_n: f64) -> Self { - // FIXME: This is very broken! - //return Self(RatioType::from_float(n).unwrap() * RatioType::from_integer(100000).round() / RatioType::from_integer(100000)); - todo!(); - } -} - impl fmt::Display for Rational { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(precision) = f.precision() { diff --git a/src/numbers/rational_rug.rs b/src/numbers/rational_rug.rs index c44707b..37ef765 100644 --- a/src/numbers/rational_rug.rs +++ b/src/numbers/rational_rug.rs @@ -107,13 +107,6 @@ impl From for Rational { fn from(n: usize) -> Self { Self(rug::Rational::from(n)) } } -impl From for Rational { - fn from(n: f64) -> Self { - // FIXME: This is very broken! - return Self((rug::Rational::from_f64(n).unwrap() * rug::Rational::from(100000)).round() / rug::Rational::from(100000)); - } -} - impl fmt::Display for Rational { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(precision) = f.precision() { diff --git a/src/stv/gregory.rs b/src/stv/gregory.rs index 662fc3f..6f604dd 100644 --- a/src/stv/gregory.rs +++ b/src/stv/gregory.rs @@ -18,8 +18,9 @@ use super::{ExclusionMethod, NextPreferencesEntry, NextPreferencesResult, STVError, STVOptions, SumSurplusTransfersMode, SurplusMethod, SurplusOrder}; use crate::constraints; -use crate::election::{Candidate, CandidateState, CountCard, CountState, Parcel, Vote}; +use crate::election::{Candidate, CandidateState, CountState, Parcel, Vote}; use crate::numbers::Number; +use crate::ties; use itertools::Itertools; @@ -62,15 +63,16 @@ where for<'r> &'r N: ops::Neg { let quota = state.quota.as_ref().unwrap(); - let mut has_surplus: Vec<(&Candidate, &CountCard)> = state.election.candidates.iter() // Present in order in case of tie - .map(|c| (c, state.candidates.get(c).unwrap())) - //.filter(|(_, cc)| &cc.votes > quota) - .filter(|(_, cc)| &cc.votes > quota && cc.parcels.iter().any(|p| !p.is_empty())) + let has_surplus: Vec<&Candidate> = state.election.candidates.iter() // Present in order in case of tie + .filter(|c| { + let cc = &state.candidates[c]; + &cc.votes > quota && cc.parcels.iter().any(|p| !p.is_empty()) + }) .collect(); if !has_surplus.is_empty() { let total_surpluses = has_surplus.iter() - .fold(N::new(), |acc, (_, cc)| acc + &cc.votes - quota); + .fold(N::new(), |acc, c| acc + &state.candidates[c].votes - quota); // Determine if surplues can be deferred if opts.defer_surpluses { @@ -80,27 +82,20 @@ where } } - match opts.surplus_order { + // Distribute top candidate's surplus + let max_cands = match opts.surplus_order { SurplusOrder::BySize => { - // Compare b with a to sort high-low - has_surplus.sort_by(|a, b| b.1.votes.cmp(&a.1.votes)); + ties::multiple_max_by(&has_surplus, |c| &state.candidates[c].votes) } SurplusOrder::ByOrder => { - has_surplus.sort_by(|a, b| a.1.order_elected.cmp(&b.1.order_elected)); + ties::multiple_min_by(&has_surplus, |c| state.candidates[c].order_elected) } - } - - // Distribute top candidate's surplus - let elected_candidate; - - // Handle ties - if has_surplus.len() > 1 && has_surplus[0].1.votes == has_surplus[1].1.votes { - let max_votes = &has_surplus[0].1.votes; - let has_surplus = has_surplus.into_iter().filter_map(|(c, cc)| if &cc.votes == max_votes { Some(c) } else { None }).collect(); - elected_candidate = super::choose_highest(state, opts, has_surplus)?; + }; + let elected_candidate = if max_cands.len() > 1 { + super::choose_highest(state, opts, max_cands)? } else { - elected_candidate = has_surplus[0].0; - } + max_cands[0] + }; distribute_surplus(state, &opts, elected_candidate); @@ -230,14 +225,14 @@ where { state.logger.log_literal(format!("Surplus of {} distributed.", elected_candidate.name)); - let count_card = state.candidates.get(elected_candidate).unwrap(); + let count_card = &state.candidates[elected_candidate]; let surplus = &count_card.votes - state.quota.as_ref().unwrap(); let votes; match opts.surplus { SurplusMethod::WIG | SurplusMethod::UIG => { // Inclusive Gregory - votes = state.candidates.get(elected_candidate).unwrap().parcels.concat(); + votes = count_card.parcels.concat(); } SurplusMethod::EG => { // Exclusive Gregory @@ -274,9 +269,17 @@ where } if opts.transferable_only { - state.logger.log_literal(format!("Transferring {:.0} transferable ballots, totalling {:.dps$} transferable votes, with surplus fraction {:.dps2$}.", &result.total_ballots - &result.exhausted.num_ballots, transferable_votes, surplus_fraction.as_ref().unwrap(), dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + if &result.total_ballots - &result.exhausted.num_ballots == N::one() { + state.logger.log_literal(format!("Transferring 1 transferable ballot, totalling {:.dps$} transferable votes, with surplus fraction {:.dps2$}.", transferable_votes, surplus_fraction.as_ref().unwrap(), dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + } else { + state.logger.log_literal(format!("Transferring {:.0} transferable ballots, totalling {:.dps$} transferable votes, with surplus fraction {:.dps2$}.", &result.total_ballots - &result.exhausted.num_ballots, transferable_votes, surplus_fraction.as_ref().unwrap(), dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + } } else { - state.logger.log_literal(format!("Transferring {:.0} ballots, totalling {:.dps$} votes, with surplus fraction {:.dps2$}.", result.total_ballots, result.total_votes, surplus_fraction.as_ref().unwrap(), dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + if result.total_ballots == N::one() { + state.logger.log_literal(format!("Transferring 1 ballot, totalling {:.dps$} votes, with surplus fraction {:.dps2$}.", result.total_votes, surplus_fraction.as_ref().unwrap(), dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + } else { + state.logger.log_literal(format!("Transferring {:.0} ballots, totalling {:.dps$} votes, with surplus fraction {:.dps2$}.", result.total_ballots, result.total_votes, surplus_fraction.as_ref().unwrap(), dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + } } } None => { @@ -389,7 +392,7 @@ where ExclusionMethod::ByValue => { // Exclude by value let max_value = excluded_candidates.iter() - .map(|c| state.candidates.get(c).unwrap().parcels.iter() + .map(|c| state.candidates[c].parcels.iter() .map(|p| p.iter().map(|v| &v.value / &v.ballot.orig_value).max().unwrap()) .max().unwrap()) .max().unwrap(); @@ -451,9 +454,17 @@ where let result = super::next_preferences(state, votes); if let ExclusionMethod::SingleStage = opts.exclusion { - state.logger.log_literal(format!("Transferring {:.0} ballot papers, totalling {:.dps$} votes.", result.total_ballots, result.total_votes, dps=opts.pp_decimals)); + if result.total_ballots == N::one() { + state.logger.log_literal(format!("Transferring 1 ballot, totalling {:.dps$} votes.", result.total_votes, dps=opts.pp_decimals)); + } else { + state.logger.log_literal(format!("Transferring {:.0} ballots, totalling {:.dps$} votes.", result.total_ballots, result.total_votes, dps=opts.pp_decimals)); + } } else { - state.logger.log_literal(format!("Transferring {:.0} ballot papers, totalling {:.dps$} votes, received at value {:.dps2$}.", result.total_ballots, result.total_votes, value, dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + if result.total_ballots == N::one() { + state.logger.log_literal(format!("Transferring 1 ballot, totalling {:.dps$} votes, received at value {:.dps2$}.", result.total_votes, value, dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + } else { + state.logger.log_literal(format!("Transferring {:.0} ballots, totalling {:.dps$} votes, received at value {:.dps2$}.", result.total_ballots, result.total_votes, value, dps=opts.pp_decimals, dps2=max(opts.pp_decimals, 2))); + } } // Transfer candidate votes diff --git a/src/stv/meek.rs b/src/stv/meek.rs index 0cbfeca..ea83dd0 100644 --- a/src/stv/meek.rs +++ b/src/stv/meek.rs @@ -211,7 +211,7 @@ fn recompute_keep_values<'s, N: Number>(state: &mut CountState<'s, N>, opts: &ST } /// Determine if the specified surpluses should be distributed, according to [STVOptions::meek_surplus_tolerance] -fn should_distribute_surpluses(state: &CountState, has_surplus: &Vec<&Candidate>, opts: &STVOptions) -> bool +fn should_distribute_surpluses<'a, N: Number>(state: &CountState<'a, N>, has_surplus: &Vec<&'a Candidate>, opts: &STVOptions) -> bool where for<'r> &'r N: ops::Sub<&'r N, Output=N>, for<'r> &'r N: ops::Div<&'r N, Output=N>, @@ -220,14 +220,14 @@ where // Distribute if any candidate has a surplus exceeding the tolerance let quota_tolerance = N::parse(&opts.meek_surplus_tolerance[0..opts.meek_surplus_tolerance.len()-1]) / N::from(100) + N::one(); return has_surplus.iter().any(|c| { - let count_card = state.candidates.get(c).unwrap(); + let count_card = &state.candidates[c]; return &count_card.votes / state.quota.as_ref().unwrap() > quota_tolerance; }); } else { // Distribute if the total surplus exceeds the tolerance let quota_tolerance = N::parse(&opts.meek_surplus_tolerance); let total_surpluses = has_surplus.iter() - .fold(N::new(), |acc, c| acc + &state.candidates.get(c).unwrap().votes - state.quota.as_ref().unwrap()); + .fold(N::new(), |acc, c| acc + &state.candidates[c].votes - state.quota.as_ref().unwrap()); return total_surpluses > quota_tolerance; } } @@ -242,7 +242,7 @@ where let quota = state.quota.as_ref().unwrap(); let mut has_surplus: Vec<&Candidate> = state.election.candidates.iter() .filter(|c| { - let count_card = state.candidates.get(c).unwrap(); + let count_card = &state.candidates[c]; return count_card.state == CandidateState::Elected && &count_card.votes > quota; }) .collect(); @@ -252,7 +252,7 @@ where // Determine if surplues can be deferred if opts.defer_surpluses { let total_surpluses = has_surplus.iter() - .fold(N::new(), |acc, c| acc + &state.candidates.get(c).unwrap().votes - quota); + .fold(N::new(), |acc, c| acc + &state.candidates[c].votes - quota); if super::can_defer_surpluses(state, opts, &total_surpluses) { state.logger.log_literal(format!("Distribution of surpluses totalling {:.dps$} votes will be deferred.", total_surpluses, dps=opts.pp_decimals)); return Ok(false); @@ -290,7 +290,7 @@ where let quota = state.quota.as_ref().unwrap(); has_surplus = state.election.candidates.iter() .filter(|c| { - let count_card = state.candidates.get(c).unwrap(); + let count_card = &state.candidates[c]; return count_card.state == CandidateState::Elected && &count_card.votes > quota; }) .collect(); @@ -300,7 +300,7 @@ where // Determine if surplues can be deferred if should_distribute && opts.defer_surpluses { let total_surpluses = has_surplus.iter() - .fold(N::new(), |acc, c| acc + &state.candidates.get(c).unwrap().votes - quota); + .fold(N::new(), |acc, c| acc + &state.candidates[c].votes - quota); if super::can_defer_surpluses(state, opts, &total_surpluses) { surpluses_deferred = Some(total_surpluses); break; @@ -311,7 +311,7 @@ where // Recalculate transfers let mut checksum = N::new(); for (candidate, count_card) in state.candidates.iter_mut() { - count_card.transfers = &count_card.votes - &orig_candidates.get(candidate).unwrap().votes; + count_card.transfers = &count_card.votes - &orig_candidates[candidate].votes; checksum += &count_card.transfers; } state.exhausted.transfers = &state.exhausted.votes - &orig_exhausted.votes; @@ -339,7 +339,7 @@ where } let kv_str = state.election.candidates.iter() - .map(|c| (c, state.candidates.get(c).unwrap())) + .map(|c| (c, &state.candidates[c])) .filter(|(_, cc)| cc.state == CandidateState::Elected) .sorted_unstable_by(|a, b| a.1.order_elected.cmp(&b.1.order_elected)) .map(|(c, cc)| format!("{} ({:.dps2$})", c.name, cc.keep_value.as_ref().unwrap(), dps2=max(opts.pp_decimals, 2))) @@ -363,14 +363,14 @@ where let quota = state.quota.as_ref().unwrap(); let has_surplus: Vec<&Candidate> = state.election.candidates.iter() .filter(|c| { - let count_card = state.candidates.get(c).unwrap(); + let count_card = &state.candidates[c]; return count_card.state == CandidateState::Elected && &count_card.votes > quota; }) .collect(); recompute_keep_values(state, opts, &has_surplus); let kv_str = state.election.candidates.iter() - .map(|c| (c, state.candidates.get(c).unwrap())) + .map(|c| (c, &state.candidates[c])) .filter(|(_, cc)| cc.state == CandidateState::Elected) .sorted_unstable_by(|a, b| a.1.order_elected.cmp(&b.1.order_elected)) .map(|(c, cc)| format!("{} ({:.dps2$})", c.name, cc.keep_value.as_ref().unwrap(), dps2=max(opts.pp_decimals, 2))) @@ -400,7 +400,7 @@ where // Recalculate transfers let mut checksum = N::new(); for (candidate, count_card) in state.candidates.iter_mut() { - count_card.transfers = &count_card.votes - &orig_candidates.get(candidate).unwrap().votes; + count_card.transfers = &count_card.votes - &orig_candidates[candidate].votes; checksum += &count_card.transfers; } state.exhausted.transfers = &state.exhausted.votes - &orig_exhausted.votes; diff --git a/src/stv/mod.rs b/src/stv/mod.rs index 3cfa068..61701a1 100644 --- a/src/stv/mod.rs +++ b/src/stv/mod.rs @@ -30,7 +30,7 @@ use crate::constraints; use crate::numbers::Number; use crate::election::{Candidate, CandidateState, CountCard, CountState, Vote}; use crate::sharandom::SHARandom; -use crate::ties::TieStrategy; +use crate::ties::{self, TieStrategy}; use itertools::Itertools; use wasm_bindgen::prelude::wasm_bindgen; @@ -565,7 +565,7 @@ fn next_preferences<'a, N: Number>(state: &CountState<'a, N>, votes: Vec(quota: &N, count_card: &CountCard, opts: &STVOption } /// Declare elected all candidates meeting the quota -fn elect_meeting_quota(state: &mut CountState, opts: &STVOptions) -> Result { +fn elect_meeting_quota<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOptions) -> Result { let vote_req = state.vote_required_election.as_ref().unwrap().clone(); // Have to do this or else the borrow checker gets confused - let mut cands_meeting_quota: Vec<&Candidate> = state.election.candidates.iter() // Present in order in case of tie - .filter(|c| { - let cc = state.candidates.get(c).unwrap(); - return (cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded) && meets_quota(&vote_req, cc, opts); - }) + let mut cands_meeting_quota: Vec<(&Candidate, &CountCard)> = state.election.candidates.iter() // Present in order in case of tie + .map(|c| (c, &state.candidates[c])) + .filter(|(_, cc)| { (cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded) && meets_quota(&vote_req, cc, opts) }) .collect(); // Sort by votes - cands_meeting_quota.sort_unstable_by(|a, b| state.candidates.get(a).unwrap().votes.cmp(&state.candidates.get(b).unwrap().votes)); + cands_meeting_quota.sort_unstable_by(|a, b| a.1.votes.cmp(&b.1.votes)); + let mut cands_meeting_quota: Vec<&Candidate> = cands_meeting_quota.iter().map(|(c, _)| *c).collect(); let elected = !cands_meeting_quota.is_empty(); while !cands_meeting_quota.is_empty() { // Declare elected in descending order of votes - let max_votes = cands_meeting_quota.iter() - .max_by(|a, b| state.candidates.get(**a).unwrap().votes.cmp(&state.candidates.get(**b).unwrap().votes)) - .unwrap(); - let max_votes = &state.candidates.get(max_votes).unwrap().votes; - let max_cands_meeting_quota: Vec<&Candidate> = cands_meeting_quota.iter() - .filter(|c| &state.candidates.get(**c).unwrap().votes == max_votes) - .map(|c| *c) - .collect(); - - let candidate; - if max_cands_meeting_quota.len() > 1 { - // Handle ties - candidate = choose_highest(state, opts, max_cands_meeting_quota)?; + let max_cands = ties::multiple_max_by(&cands_meeting_quota, |c| &state.candidates[c].votes); + let candidate = if max_cands.len() > 1 { + choose_highest(state, opts, max_cands)? } else { - candidate = max_cands_meeting_quota[0]; - } + max_cands[0] + }; let count_card = state.candidates.get_mut(candidate).unwrap(); count_card.state = CandidateState::Elected; @@ -788,10 +777,12 @@ fn elect_meeting_quota(state: &mut CountState, opts: &STVOptions) if constraints::update_constraints(state, opts) { // Recheck as some candidates may have been doomed - cands_meeting_quota = state.election.candidates.iter() - .filter(|c| { let cc = state.candidates.get(c).unwrap(); cc.state == CandidateState::Hopeful && meets_quota(&vote_req, cc, opts) }) + let mut cmq: Vec<(&Candidate, &CountCard)> = state.election.candidates.iter() // Present in order in case of tie + .map(|c| (c, &state.candidates[c])) + .filter(|(_, cc)| { (cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded) && meets_quota(&vote_req, cc, opts) }) .collect(); - cands_meeting_quota.sort_unstable_by(|a, b| state.candidates.get(a).unwrap().votes.cmp(&state.candidates.get(b).unwrap().votes)); + cmq.sort_unstable_by(|a, b| a.1.votes.cmp(&b.1.votes)); + cands_meeting_quota = cmq.iter().map(|(c, _)| *c).collect(); } else { cands_meeting_quota.remove(cands_meeting_quota.iter().position(|c| *c == candidate).unwrap()); } @@ -835,7 +826,7 @@ where let num_to_exclude = to_exclude.len(); if num_to_exclude > 0 { let total_excluded = to_exclude.into_iter() - .fold(N::new(), |acc, c| acc + &state.candidates.get(c).unwrap().votes); + .fold(N::new(), |acc, c| acc + &state.candidates[c].votes); if total_surpluses > &(&hopefuls[num_to_exclude + 1].1.votes - &total_excluded) { return false; } @@ -866,7 +857,7 @@ where fn bulk_elect(state: &mut CountState, opts: &STVOptions) -> Result { let mut hopefuls: Vec<&Candidate> = state.election.candidates.iter() .filter(|c| { - let cc = state.candidates.get(c).unwrap(); + let cc = &state.candidates[c]; return cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded; }) .collect(); @@ -877,22 +868,12 @@ fn bulk_elect(state: &mut CountState, opts: &STVOptions) -> Result // Bulk elect all remaining candidates while !hopefuls.is_empty() { - let max_votes = hopefuls.iter() - .max_by(|a, b| state.candidates.get(**a).unwrap().votes.cmp(&state.candidates.get(**b).unwrap().votes)) - .unwrap(); - let max_votes = &state.candidates.get(max_votes).unwrap().votes; - let max_hopefuls: Vec<&Candidate> = hopefuls.iter() - .filter(|c| &state.candidates.get(**c).unwrap().votes == max_votes) - .map(|c| *c) - .collect(); - - let candidate; - if max_hopefuls.len() > 1 { - // Handle ties - candidate = choose_highest(state, opts, max_hopefuls)?; + let max_cands = ties::multiple_max_by(&hopefuls, |c| &state.candidates[c].votes); + let candidate = if max_cands.len() > 1 { + choose_highest(state, opts, max_cands)? } else { - candidate = max_hopefuls[0]; - } + max_cands[0] + }; let count_card = state.candidates.get_mut(candidate).unwrap(); count_card.state = CandidateState::Elected; @@ -909,7 +890,7 @@ fn bulk_elect(state: &mut CountState, opts: &STVOptions) -> Result // Recheck as some candidates may have been doomed hopefuls = state.election.candidates.iter() .filter(|c| { - let cc = state.candidates.get(c).unwrap(); + let cc = &state.candidates[c]; return cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded; }) .collect(); @@ -929,29 +910,22 @@ where for<'r> &'r N: ops::Mul<&'r N, Output=N>, for<'r> &'r N: ops::Div<&'r N, Output=N>, { - let mut doomed: Vec<(&Candidate, &CountCard)> = state.election.candidates.iter() // Present in order in case of tie - .map(|c| (c, state.candidates.get(c).unwrap())) - .filter(|(_, cc)| cc.state == CandidateState::Doomed) + let doomed: Vec<&Candidate> = state.election.candidates.iter() // Present in order in case of tie + .filter(|c| state.candidates[c].state == CandidateState::Doomed) .collect(); if !doomed.is_empty() { let excluded_candidates; - if opts.bulk_exclude { - excluded_candidates = doomed.into_iter().map(|(c, _)| c).collect(); + excluded_candidates = doomed; } else { // Exclude only the lowest-ranked doomed candidate - // Sort by votes - doomed.sort_by(|a, b| a.1.votes.cmp(&b.1.votes)); - - // Handle ties - if doomed.len() > 1 && doomed[0].1.votes == doomed[1].1.votes { - let min_votes = &doomed[0].1.votes; - let doomed = doomed.into_iter().filter_map(|(c, cc)| if &cc.votes == min_votes { Some(c) } else { None }).collect(); - excluded_candidates = vec![choose_lowest(state, opts, doomed)?]; + let min_cands = ties::multiple_min_by(&doomed, |c| &state.candidates[c].votes); + excluded_candidates = if min_cands.len() > 1 { + vec![choose_lowest(state, opts, min_cands)?] } else { - excluded_candidates = vec![&doomed[0].0]; - } + vec![min_cands[0]] + }; } let names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).sorted().collect(); @@ -1028,22 +1002,16 @@ where // Exclude lowest ranked candidate if excluded_candidates.is_empty() { - let mut hopefuls: Vec<(&Candidate, &CountCard)> = state.election.candidates.iter() // Present in order in case of tie - .map(|c| (c, state.candidates.get(c).unwrap())) - .filter(|(_, cc)| cc.state == CandidateState::Hopeful) + let hopefuls: Vec<&Candidate> = state.election.candidates.iter() // Present in order in case of tie + .filter(|c| state.candidates[c].state == CandidateState::Hopeful) .collect(); - // Sort by votes - hopefuls.sort_by(|a, b| a.1.votes.cmp(&b.1.votes)); - - // Handle ties - if hopefuls.len() > 1 && hopefuls[0].1.votes == hopefuls[1].1.votes { - let min_votes = &hopefuls[0].1.votes; - let hopefuls = hopefuls.into_iter().filter_map(|(c, cc)| if &cc.votes == min_votes { Some(c) } else { None }).collect(); - excluded_candidates = vec![choose_lowest(state, opts, hopefuls)?]; + let min_cands = ties::multiple_min_by(&hopefuls, |c| &state.candidates[c].votes); + excluded_candidates = if min_cands.len() > 1 { + vec![choose_lowest(state, opts, min_cands)?] } else { - excluded_candidates = vec![&hopefuls[0].0]; - } + vec![min_cands[0]] + }; } let names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).sorted().collect(); @@ -1278,9 +1246,9 @@ fn update_tiebreaks(state: &mut CountState, _opts: &STVOptions) { } else { // Tied in this round - refer to last round let mut tied_this_round: Vec<&Candidate> = group.into_iter().map(|c| *c).collect(); - tied_this_round.sort_unstable_by(|a, b| hm_orig.get(a).unwrap().cmp(hm_orig.get(b).unwrap())); + tied_this_round.sort_unstable_by(|a, b| hm_orig[a].cmp(&hm_orig[b])); let tied_this_round = tied_this_round.into_iter() - .group_by(|c| hm_orig.get(c).unwrap()); + .group_by(|c| hm_orig[c]); for (_, group2) in tied_this_round.into_iter() { for candidate in group2 { diff --git a/src/stv/wasm.rs b/src/stv/wasm.rs index 248c7b0..13a972c 100644 --- a/src/stv/wasm.rs +++ b/src/stv/wasm.rs @@ -314,7 +314,7 @@ fn update_results_table(stage_num: usize, state: &CountState, opts result.push(&format!(r#"{}"#, tdclasses1, state.kind.unwrap_or("")).into()); result.push(&format!(r#"{}"#, tdclasses1, state.title).into()); for candidate in state.election.candidates.iter() { - let count_card = state.candidates.get(candidate).unwrap(); + let count_card = &state.candidates[candidate]; match count_card.state { CandidateState::Hopeful | CandidateState::Guarded => { result.push(&format!(r#"{}"#, tdclasses2, pp(&count_card.transfers, opts.pp_decimals)).into()); @@ -377,7 +377,7 @@ fn finalise_results_table(state: &CountState) -> Array { // Candidate states for candidate in state.election.candidates.iter() { - let count_card = state.candidates.get(candidate).unwrap(); + let count_card = &state.candidates[candidate]; if count_card.state == stv::CandidateState::Elected { result.push(&format!(r#"ELECTED {}"#, count_card.order_elected).into()); } else if count_card.state == stv::CandidateState::Excluded { @@ -403,7 +403,7 @@ fn final_result_summary(state: &CountState, opts: &stv::STVOptions winners.push((candidate, count_card.order_elected, &count_card.keep_value)); } } - winners.sort_unstable_by(|a, b| a.1.partial_cmp(&b.1).unwrap()); + winners.sort_unstable_by(|a, b| a.1.cmp(&b.1)); for (winner, _, kv_opt) in winners.into_iter() { if let Some(kv) = kv_opt { diff --git a/src/ties.rs b/src/ties.rs index 809d24d..3afe172 100644 --- a/src/ties.rs +++ b/src/ties.rs @@ -59,10 +59,10 @@ impl TieStrategy { let mut candidates = candidates.clone(); candidates.sort_unstable_by(|a, b| // Compare b to a to sort high-to-low - state.forwards_tiebreak.as_ref().unwrap().get(b).unwrap() - .cmp(&state.forwards_tiebreak.as_ref().unwrap().get(a).unwrap()) + state.forwards_tiebreak.as_ref().unwrap()[b] + .cmp(&state.forwards_tiebreak.as_ref().unwrap()[a]) ); - if state.forwards_tiebreak.as_ref().unwrap().get(candidates[0]).unwrap() == state.forwards_tiebreak.as_ref().unwrap().get(candidates[1]).unwrap() { + if state.forwards_tiebreak.as_ref().unwrap()[candidates[0]] == state.forwards_tiebreak.as_ref().unwrap()[candidates[1]] { return Err(STVError::UnresolvedTie); } else { state.logger.log_literal(format!("Tie between {} broken forwards.", smart_join(&candidates.iter().map(|c| c.name.as_str()).collect()))); @@ -72,10 +72,10 @@ impl TieStrategy { Self::Backwards => { let mut candidates = candidates.clone(); candidates.sort_unstable_by(|a, b| - state.backwards_tiebreak.as_ref().unwrap().get(b).unwrap() - .cmp(&state.backwards_tiebreak.as_ref().unwrap().get(a).unwrap()) + state.backwards_tiebreak.as_ref().unwrap()[b] + .cmp(&state.backwards_tiebreak.as_ref().unwrap()[a]) ); - if state.backwards_tiebreak.as_ref().unwrap().get(candidates[0]).unwrap() == state.backwards_tiebreak.as_ref().unwrap().get(candidates[1]).unwrap() { + if state.backwards_tiebreak.as_ref().unwrap()[candidates[0]] == state.backwards_tiebreak.as_ref().unwrap()[candidates[1]] { return Err(STVError::UnresolvedTie); } else { state.logger.log_literal(format!("Tie between {} broken backwards.", smart_join(&candidates.iter().map(|c| c.name.as_str()).collect()))); @@ -106,10 +106,10 @@ impl TieStrategy { Self::Forwards => { let mut candidates = candidates.clone(); candidates.sort_unstable_by(|a, b| - state.forwards_tiebreak.as_ref().unwrap().get(a).unwrap() - .cmp(&state.forwards_tiebreak.as_ref().unwrap().get(b).unwrap()) + state.forwards_tiebreak.as_ref().unwrap()[a] + .cmp(&state.forwards_tiebreak.as_ref().unwrap()[b]) ); - if state.forwards_tiebreak.as_ref().unwrap().get(candidates[0]).unwrap() == state.forwards_tiebreak.as_ref().unwrap().get(candidates[1]).unwrap() { + if state.forwards_tiebreak.as_ref().unwrap()[candidates[0]] == state.forwards_tiebreak.as_ref().unwrap()[candidates[1]] { return Err(STVError::UnresolvedTie); } else { state.logger.log_literal(format!("Tie between {} broken forwards.", smart_join(&candidates.iter().map(|c| c.name.as_str()).collect()))); @@ -119,10 +119,10 @@ impl TieStrategy { Self::Backwards => { let mut candidates = candidates.clone(); candidates.sort_unstable_by(|a, b| - state.backwards_tiebreak.as_ref().unwrap().get(a).unwrap() - .cmp(&state.backwards_tiebreak.as_ref().unwrap().get(b).unwrap()) + state.backwards_tiebreak.as_ref().unwrap()[a] + .cmp(&state.backwards_tiebreak.as_ref().unwrap()[b]) ); - if state.backwards_tiebreak.as_ref().unwrap().get(candidates[0]).unwrap() == state.backwards_tiebreak.as_ref().unwrap().get(candidates[1]).unwrap() { + if state.backwards_tiebreak.as_ref().unwrap()[candidates[0]] == state.backwards_tiebreak.as_ref().unwrap()[candidates[1]] { return Err(STVError::UnresolvedTie); } else { state.logger.log_literal(format!("Tie between {} broken backwards.", smart_join(&candidates.iter().map(|c| c.name.as_str()).collect()))); @@ -139,6 +139,64 @@ impl TieStrategy { } } +/// Return all maximal items according to the given key +pub fn multiple_max_by(items: &Vec, key: K) -> Vec +where + K: Fn(&E) -> C +{ + let mut max_key = None; + let mut max_items = Vec::new(); + for item in items.iter() { + let item_key = key(item); + match &max_key { + Some(v) => { + if &item_key > v { + max_key = Some(item_key); + max_items.clear(); + max_items.push(*item); + } else if &item_key == v { + max_items.push(*item); + } + } + None => { + max_key = Some(item_key); + max_items.clear(); + max_items.push(*item); + } + } + } + return max_items; +} + +/// Return all minimal items according to the given key +pub fn multiple_min_by(items: &Vec, key: K) -> Vec +where + K: Fn(&E) -> C +{ + let mut min_key = None; + let mut min_items = Vec::new(); + for item in items.iter() { + let item_key = key(item); + match &min_key { + Some(v) => { + if &item_key < v { + min_key = Some(item_key); + min_items.clear(); + min_items.push(*item); + } else if &item_key == v { + min_items.push(*item); + } + } + None => { + min_key = Some(item_key); + min_items.clear(); + min_items.push(*item); + } + } + } + return min_items; +} + /// Prompt the candidate for input, depending on CLI or WebAssembly target #[cfg(not(target_arch = "wasm32"))] fn prompt<'c>(candidates: &Vec<&'c Candidate>) -> Result<&'c Candidate, STVError> { diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs index 9a73c9e..062d316 100644 --- a/tests/utils/mod.rs +++ b/tests/utils/mod.rs @@ -106,7 +106,7 @@ where } let mut candidate_votes: Vec> = records.iter().skip(2) - .map(|r| if r[idx*2 + 1].len() > 0 { Some(N::from(r[idx*2 + 1].parse::().expect("Syntax Error"))) } else { None }) + .map(|r| if r[idx*2 + 1].len() > 0 { Some(N::parse(&r[idx*2 + 1])) } else { None }) .collect(); // Validate exhausted/LBF