Clean up: candidate manipulation returns a Result.

Functions to manipulate candidates during the solution search now
always return a Result instead of sometimes returning a bool, and
other times returning an Option.

We opted to use Result over Option mainly to (ab)use the try! macro.
In addition, it also makes it emit a warning when we forget to handle
the result.  (We tend to assume people don't just ignore the result
and continue as normal, instead of putting the puzzle in a special
contradiction detected state.)

src/constraint/alldifferent.rs

@@ -3,7 +3,7 @@
 use std::collections::HashMap;
 use std::rc::Rc;
 
-use ::{Constraint,PuzzleSearch,Val,VarToken};
+use ::{Constraint,PsResult,PuzzleSearch,Val,VarToken};
 
 pub struct AllDifferent {
     vars: Vec<VarToken>,
@@ -34,18 +34,16 @@ impl Constraint for AllDifferent {
         Box::new(self.vars.iter())
     }
 
-    fn on_assigned(&self, search: &mut PuzzleSearch, _var: VarToken, val: Val)
+    fn on_assigned(&self, search: &mut PuzzleSearch, var: VarToken, val: Val)
-            -> bool {
+            -> PsResult<()> {
-        for &var2 in self.vars.iter() {
+        for &var2 in self.vars.iter().filter(|&v| *v != var) {
-            if !search.is_assigned(var2) {
+            try!(search.remove_candidate(var2, val));
-                search.remove_candidate(var2, val);
-            }
         }
 
-        true
+        Ok(())
     }
 
-    fn on_updated(&self, search: &mut PuzzleSearch) -> bool {
+    fn on_updated(&self, search: &mut PuzzleSearch) -> PsResult<()> {
         // Build a table of which values can be assigned to which variables.
         let mut num_unassigned = 0;
         let mut all_candidates = HashMap::new();
@@ -64,30 +62,30 @@ impl Constraint for AllDifferent {
 
         if num_unassigned > all_candidates.len() {
             // More unassigned variables than candidates, contradiction.
-            return false;
+            return Err(());
         } else if num_unassigned == all_candidates.len() {
             // As many as variables as candidates.
             for (&val, &opt) in all_candidates.iter() {
                 if let Some(var) = opt {
-                    search.set_candidate(var, val);
+                    try!(search.set_candidate(var, val));
                 }
             }
         }
 
-        true
+        Ok(())
     }
 
     fn substitute(&self, from: VarToken, to: VarToken)
-            -> Option<Rc<Constraint>> {
+            -> PsResult<Rc<Constraint>> {
         if let Some(idx) = self.vars.iter().position(|&var| var == from) {
             if !self.vars.contains(&to) {
                 let mut new_vars = self.vars.clone();
                 new_vars[idx] = to;
-                return Some(Rc::new(AllDifferent{ vars: new_vars }));
+                return Ok(Rc::new(AllDifferent{ vars: new_vars }));
             }
         }
 
-        None
+        Err(())
     }
 }
 

src/constraint/equality.rs

@@ -2,7 +2,7 @@
 
 use std::rc::Rc;
 
-use ::{Constraint,LinExpr,PuzzleSearch,Val,VarToken};
+use ::{Constraint,LinExpr,PsResult,PuzzleSearch,Val,VarToken};
 use intdiv::IntDiv;
 
 pub struct Equality {
@@ -36,7 +36,7 @@ impl Constraint for Equality {
     }
 
     fn on_assigned(&self, search: &mut PuzzleSearch, _: VarToken, _: Val)
-            -> bool {
+            -> PsResult<()> {
         let mut sum = self.eqn.constant;
         let mut unassigned_var = None;
 
@@ -47,7 +47,7 @@ impl Constraint for Equality {
                 // If we find more than one unassigned variable,
                 // cannot assign any other variables.
                 if unassigned_var.is_some() {
-                    return true;
+                    return Ok(());
                 } else {
                     unassigned_var = Some((var, coef));
                 }
@@ -59,34 +59,31 @@ impl Constraint for Equality {
             // sum + coef * var = 0.
             let val = -sum / coef;
             if sum + coef * val == 0 {
-                search.set_candidate(var, val);
+                try!(search.set_candidate(var, val));
             } else {
-                return false;
+                return Err(());
             }
         } else {
             if sum != 0 {
-                return false;
+                return Err(());
             }
         }
 
-        true
+        Ok(())
     }
 
-    fn on_updated(&self, search: &mut PuzzleSearch) -> bool {
+    fn on_updated(&self, search: &mut PuzzleSearch) -> PsResult<()> {
         let mut sum_min = self.eqn.constant;
         let mut sum_max = self.eqn.constant;
 
         for (&var, &coef) in self.eqn.coef.iter() {
-            if let Some((min_val, max_val)) = search.get_min_max(var) {
+            let (min_val, max_val) = try!(search.get_min_max(var));
-                if coef > 0 {
+            if coef > 0 {
-                    sum_min += coef * min_val;
+                sum_min += coef * min_val;
-                    sum_max += coef * max_val;
+                sum_max += coef * max_val;
-                } else {
-                    sum_min += coef * max_val;
-                    sum_max += coef * min_val;
-                }
             } else {
-                return false;
+                sum_min += coef * max_val;
+                sum_max += coef * min_val;
             }
         }
 
@@ -98,7 +95,7 @@ impl Constraint for Equality {
         while iters > 0 {
             iters = iters - 1;
             if !(sum_min <= 0 && 0 <= sum_max) {
-                return false;
+                return Err(());
             }
 
             let (&var, &coef) = iter.next().expect("cycle");
@@ -106,51 +103,44 @@ impl Constraint for Equality {
                 continue;
             }
 
-            if let Some((min_val, max_val)) = search.get_min_max(var) {
+            let (min_val, max_val) = try!(search.get_min_max(var));
-                let min_bnd;
+            let (min_bnd, max_bnd);
-                let max_bnd;
+
+            if coef > 0 {
+                min_bnd = (coef * max_val - sum_max).div_round_up(coef);
+                max_bnd = (coef * min_val - sum_min).div_round_down(coef);
+            } else {
+                min_bnd = (coef * max_val - sum_min).div_round_up(coef);
+                max_bnd = (coef * min_val - sum_max).div_round_down(coef);
+            }
+
+            if min_val < min_bnd || max_bnd < max_val {
+                let (new_min, new_max)
+                    = try!(search.bound_candidate_range(var, min_bnd, max_bnd));
 
                 if coef > 0 {
-                    min_bnd = (coef * max_val - sum_max).div_round_up(coef);
+                    sum_min = sum_min + coef * (new_min - min_val);
-                    max_bnd = (coef * min_val - sum_min).div_round_down(coef);
+                    sum_max = sum_max + coef * (new_max - max_val);
                 } else {
-                    min_bnd = (coef * max_val - sum_min).div_round_up(coef);
+                    sum_min = sum_min + coef * (new_max - max_val);
-                    max_bnd = (coef * min_val - sum_max).div_round_down(coef);
+                    sum_max = sum_max + coef * (new_min - min_val);
                 }
 
-                if min_val < min_bnd || max_bnd < max_val {
+                iters = self.eqn.coef.len();
-                    search.bound_candidate_range(var, min_bnd, max_bnd);
-
-                    if let Some((new_min, new_max)) = search.get_min_max(var) {
-                        if coef > 0 {
-                            sum_min = sum_min + coef * (new_min - min_val);
-                            sum_max = sum_max + coef * (new_max - max_val);
-                        } else {
-                            sum_min = sum_min + coef * (new_max - max_val);
-                            sum_max = sum_max + coef * (new_min - min_val);
-                        }
-                    } else {
-                        return false;
-                    }
-
-                    iters = self.eqn.coef.len();
-                }
-            } else {
-                return false;
             }
         }
 
-        true
+        Ok(())
     }
 
     fn substitute(&self, from: VarToken, to: VarToken)
-            -> Option<Rc<Constraint>> {
+            -> PsResult<Rc<Constraint>> {
         let mut eqn = self.eqn.clone();
         if let Some(coef) = eqn.coef.remove(&from) {
             eqn = eqn + coef * to;
         }
 
-        Some(Rc::new(Equality{ eqn: eqn }))
+        Ok(Rc::new(Equality{ eqn: eqn }))
     }
 }
 

src/constraint/mod.rs

@@ -7,7 +7,7 @@
 
 use std::rc::Rc;
 
-use ::{PuzzleSearch,Val,VarToken};
+use ::{PsResult,PuzzleSearch,Val,VarToken};
 
 /// Constraint trait.
 pub trait Constraint {
@@ -15,29 +15,22 @@ pub trait Constraint {
     fn vars<'a>(&'a self) -> Box<Iterator<Item=&'a VarToken> + 'a>;
 
     /// Applied after a variable has been assigned.
-    ///
-    /// Returns true if the search should continue with these variable
-    /// assignments, or false if the constraint found a contradiction.
     fn on_assigned(&self, _search: &mut PuzzleSearch, _var: VarToken, _val: Val)
-            -> bool {
+            -> PsResult<()> {
-        true
+        Ok(())
     }
 
     /// Applied after a variable's candidates has been modified.
-    ///
+    fn on_updated(&self, _search: &mut PuzzleSearch) -> PsResult<()> {
-    /// Returns true if the search should continue with these variable
+        Ok(())
-    /// assignments, or false if the constraint found a contradiction.
-    fn on_updated(&self, _search: &mut PuzzleSearch) -> bool {
-        true
     }
 
     /// Substitute the "from" variable with the "to" variable.
     ///
     /// Returns a new constraint with all instances of "from" replaced
-    /// with "to", or None if a contradiction was found in the
+    /// with "to", or Err if a contradiction was found.
-    /// process.
     fn substitute(&self, from: VarToken, to: VarToken)
-            -> Option<Rc<Constraint>>;
+            -> PsResult<Rc<Constraint>>;
 }
 
 pub use self::alldifferent::AllDifferent;

src/lib.rs

@@ -35,6 +35,9 @@ pub struct LinExpr {
     coef: HashMap<VarToken, Coef>,
 }
 
+/// A result during a puzzle solution search (Err = contradiction).
+pub type PsResult<T> = Result<T, ()>;
+
 /// A dictionary mapping puzzle variables to the solution value.
 #[derive(Debug)]
 pub struct Solution {

src/puzzle.rs

@@ -9,7 +9,7 @@ use std::ops::Index;
 use std::rc::Rc;
 use bit_set::BitSet;
 
-use ::{Constraint,LinExpr,Solution,Val,VarToken};
+use ::{Constraint,LinExpr,PsResult,Solution,Val,VarToken};
 use constraint;
 
 /// A collection of candidates.
@@ -219,7 +219,6 @@ impl Puzzle {
             &Candidates::Set(_) => (),
         }
 
-        // Why you dumb Rust?
         if let Candidates::Set(ref mut rc) = self.candidates[idx] {
             let cs = Rc::get_mut(rc).expect("unique");
             cs.extend(candidates);
@@ -417,7 +416,7 @@ impl Puzzle {
     pub fn step(&mut self) -> Option<PuzzleSearch> {
         if self.num_vars > 0 {
             let mut search = PuzzleSearch::new(self);
-            if search.constrain() {
+            if search.constrain().is_ok() {
                 return Some(search);
             }
         }
@@ -486,105 +485,131 @@ impl<'a> PuzzleSearch<'a> {
     }
 
     /// Get the minimum and maximum values for variable.
-    pub fn get_min_max(&self, var: VarToken) -> Option<(Val, Val)> {
+    pub fn get_min_max(&self, var: VarToken) -> PsResult<(Val, Val)> {
         let VarToken(idx) = var;
         match &self.vars[idx] {
-            &VarState::Assigned(val) => Some((val, val)),
+            &VarState::Assigned(val) => Ok((val, val)),
             &VarState::Unassigned(ref cs) => match cs {
-                &Candidates::None => None,
+                &Candidates::None => Err(()),
-                &Candidates::Value(val) => Some((val, val)),
+                &Candidates::Value(val) => Ok((val, val)),
                 &Candidates::Set(ref rc) => {
                     rc.iter().cloned().min().into_iter()
                         .zip(rc.iter().cloned().max()).next()
+                        .ok_or(())
                 }
             },
         }
     }
 
-    /// Set a variable to a known value.
+    /// Set a variable to a value.
-    pub fn set_candidate(&mut self, var: VarToken, val: Val) {
+    pub fn set_candidate(&mut self, var: VarToken, val: Val)
+            -> PsResult<()> {
         let VarToken(idx) = var;
-        match &mut self.vars[idx] {
+
-            &mut VarState::Assigned(_) => (),
+        match &self.vars[idx] {
-            &mut VarState::Unassigned(ref mut cs) => match cs {
+            &VarState::Assigned(v) => return bool_to_result(v == val),
-                &mut Candidates::None => return,
+            &VarState::Unassigned(ref cs) => match cs {
-                &mut Candidates::Value(v) => {
+                &Candidates::None => return Err(()),
-                    if v != val {
+                &Candidates::Value(v) => return bool_to_result(v == val),
-                        *cs = Candidates::None;
+                &Candidates::Set(_) => (),
-                        self.wake.union_with(&self.puzzle.wake[idx]);
-                    }
-                },
-                &mut Candidates::Set(ref mut rc) => {
-                    if rc.contains(&val) {
-                        let mut set = Rc::make_mut(rc);
-                        set.clear();
-                        set.insert(val);
-                        self.wake.union_with(&self.puzzle.wake[idx]);
-                    } else {
-                        let mut set = Rc::make_mut(rc);
-                        set.clear();
-                        self.wake.union_with(&self.puzzle.wake[idx]);
-                    }
-                },
             },
         }
-    }
 
-    /// Remove a single candidate from an unassigned variable.
+        if let &mut VarState::Unassigned(Candidates::Set(ref mut rc))
-    pub fn remove_candidate(&mut self, var: VarToken, val: Val) {
+                = &mut self.vars[idx] {
-        let VarToken(idx) = var;
+            if rc.contains(&val) {
-        if let VarState::Unassigned(ref mut cs) = self.vars[idx] {
+                let mut set = Rc::make_mut(rc);
-            match cs {
+                set.clear();
-                &mut Candidates::None => return,
+                set.insert(val);
-                &mut Candidates::Value(v) => {
+                self.wake.union_with(&self.puzzle.wake[idx]);
-                    if v == val {
+                Ok(())
-                        *cs = Candidates::None;
+            } else {
-                        self.wake.union_with(&self.puzzle.wake[idx]);
+                Err(())
-                    }
-                },
-                &mut Candidates::Set(ref mut rc) => {
-                    if rc.contains(&val) {
-                        let mut set = Rc::make_mut(rc);
-                        set.remove(&val);
-                        self.wake.union_with(&self.puzzle.wake[idx]);
-                    }
-                },
             }
+        } else {
+            unreachable!();
         }
     }
 
-    /// Bound an unassigned variable to the given range.
+    /// Remove a single candidate from a variable.
-    pub fn bound_candidate_range(&mut self, var: VarToken, min: Val, max: Val) {
+    pub fn remove_candidate(&mut self, var: VarToken, val: Val)
+            -> PsResult<()> {
         let VarToken(idx) = var;
-        if let VarState::Unassigned(ref mut cs) = self.vars[idx] {
-            match cs {
-                &mut Candidates::None => return,
-                &mut Candidates::Value(v) => {
-                    if !(min <= v && v <= max) {
-                        *cs = Candidates::None;
-                        self.wake.union_with(&self.puzzle.wake[idx]);
-                    }
-                },
-                &mut Candidates::Set(ref mut rc) => {
-                    let &curr_min = rc.iter().min().expect("candidates");
-                    let &curr_max = rc.iter().max().expect("candidates");
 
-                    if curr_min < min || max < curr_max {
+        match &self.vars[idx] {
-                        let mut set = Rc::make_mut(rc);
+            &VarState::Assigned(v) => return bool_to_result(v != val),
-                        *set = set.iter()
+            &VarState::Unassigned(ref cs) => match cs {
-                            .filter(|&val| min <= *val && *val <= max)
+                &Candidates::None => return Err(()),
-                            .cloned()
+                &Candidates::Value(v) => return bool_to_result(v != val),
-                            .collect();
+                &Candidates::Set(_) => (),
-                        self.wake.union_with(&self.puzzle.wake[idx]);
+            },
-                    }
+        }
-                },
+
+        if let &mut VarState::Unassigned(Candidates::Set(ref mut rc))
+                = &mut self.vars[idx] {
+            if rc.contains(&val) {
+                let mut set = Rc::make_mut(rc);
+                set.remove(&val);
+                self.wake.union_with(&self.puzzle.wake[idx]);
             }
+            bool_to_result(!rc.is_empty())
+        } else {
+            unreachable!();
+        }
+    }
+
+    /// Bound an variable to the given range.
+    pub fn bound_candidate_range(&mut self, var: VarToken, min: Val, max: Val)
+            -> PsResult<(Val, Val)> {
+        let VarToken(idx) = var;
+
+        match &self.vars[idx] {
+            &VarState::Assigned(v) =>
+                if min <= v && v <= max {
+                    return Ok((v, v))
+                } else {
+                    return Err(())
+                },
+            &VarState::Unassigned(ref cs) => match cs {
+                &Candidates::None => return Err(()),
+                &Candidates::Value(v) =>
+                    if min <= v && v <= max {
+                        return Ok((v, v))
+                    } else {
+                        return Err(())
+                    },
+                &Candidates::Set(_) => (),
+            },
+        }
+
+        if let &mut VarState::Unassigned(Candidates::Set(ref mut rc))
+                = &mut self.vars[idx] {
+            let &curr_min = rc.iter().min().expect("candidates");
+            let &curr_max = rc.iter().max().expect("candidates");
+
+            if curr_min < min || max < curr_max {
+                {
+                    let mut set = Rc::make_mut(rc);
+                    *set = set.iter()
+                        .filter(|&val| min <= *val && *val <= max)
+                        .cloned()
+                        .collect();
+                    self.wake.union_with(&self.puzzle.wake[idx]);
+                }
+                rc.iter().cloned().min().into_iter()
+                    .zip(rc.iter().cloned().max()).next()
+                    .ok_or(())
+            } else {
+                Ok((curr_min, curr_max))
+            }
+        } else {
+            unreachable!();
         }
     }
 
     /// Solve the puzzle, finding up to count solutions.
     fn solve(&mut self, count: usize, solutions: &mut Vec<Solution>) {
-        if !self.constrain() {
+        if self.constrain().is_err() {
             return;
         }
 
@@ -605,7 +630,7 @@ impl<'a> PuzzleSearch<'a> {
                 self.puzzle.num_guesses.set(num_guesses);
 
                 let mut new = self.clone();
-                if !new.assign(idx, val) {
+                if new.assign(idx, val).is_err() {
                     continue;
                 }
 
@@ -629,29 +654,23 @@ impl<'a> PuzzleSearch<'a> {
     }
 
     /// Assign a variable (given by index) to a value.
-    ///
+    fn assign(&mut self, idx: usize, val: Val) -> PsResult<()> {
-    /// Returns false if a contradiction was found.
-    fn assign(&mut self, idx: usize, val: Val) -> bool {
         let var = VarToken(idx);
         self.vars[idx] = VarState::Assigned(val);
         self.wake.union_with(&self.puzzle.wake[idx]);
 
         for (cidx, constraint) in self.puzzle.constraints.iter().enumerate() {
             if self.puzzle.wake[idx].contains(cidx) {
-                if !constraint.on_assigned(self, var, val) {
+                try!(constraint.on_assigned(self, var, val));
-                    return false;
-                }
             }
         }
 
-        true
+        Ok(())
     }
 
     /// Take any obvious non-choices, using the constraints to
     /// eliminate candidates.  Stops when it must start guessing.
-    ///
+    fn constrain(&mut self) -> PsResult<()> {
-    /// Returns false if a contradiction was found.
-    fn constrain(&mut self) -> bool {
         while !self.wake.is_empty() {
             // "Gimme" phase:
             // - abort if any variables with 0 candidates,
@@ -664,16 +683,14 @@ impl<'a> PuzzleSearch<'a> {
                 let gimme = match &self.vars[idx] {
                     &VarState::Assigned(_) => None,
                     &VarState::Unassigned(ref cs) => match cs.len() {
-                        0 => return false,
+                        0 => return Err(()),
                         1 => cs.iter().next(),
                         _ => None,
                     }
                 };
 
                 if let Some(val) = gimme {
-                    if !self.assign(idx, val) {
+                    try!(self.assign(idx, val));
-                        return false;
-                    }
                     last_gimme = idx;
                 } else if idx == last_gimme {
                     break;
@@ -686,14 +703,12 @@ impl<'a> PuzzleSearch<'a> {
             if !self.wake.is_empty() {
                 let wake = mem::replace(&mut self.wake, BitSet::new());
                 for cidx in wake.iter() {
-                    if !self.puzzle.constraints[cidx].on_updated(self) {
+                    try!(self.puzzle.constraints[cidx].on_updated(self));
-                        return false;
-                    }
                 }
             }
         }
 
-        true
+        Ok(())
     }
 }
 
@@ -736,6 +751,14 @@ impl<'a> Index<VarToken> for PuzzleSearch<'a> {
     }
 }
 
+fn bool_to_result(cond: bool) -> PsResult<()> {
+    if cond {
+        Ok(())
+    } else {
+        Err(())
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use ::Puzzle;

tests/queens.rs

@@ -5,7 +5,7 @@
 extern crate puzzle_solver;
 
 use std::rc::Rc;
-use puzzle_solver::{Constraint,Puzzle,PuzzleSearch,Solution,Val,VarToken};
+use puzzle_solver::*;
 
 struct NoDiagonal {
     vars: Vec<VarToken>,
@@ -17,22 +17,22 @@ impl Constraint for NoDiagonal {
     }
 
     fn on_assigned(&self, search: &mut PuzzleSearch, var: VarToken, val: Val)
-            -> bool {
+            -> PsResult<()> {
         let y1 = self.vars.iter().position(|&v| v == var).expect("unreachable");
         for (y2, &var2) in self.vars.iter().enumerate() {
             if !search.is_assigned(var2) {
                 let x1 = val;
                 let dy = (y1 as Val) - (y2 as Val);
-                search.remove_candidate(var2, x1 - dy);
+                try!(search.remove_candidate(var2, x1 - dy));
-                search.remove_candidate(var2, x1 + dy);
+                try!(search.remove_candidate(var2, x1 + dy));
             }
         }
 
-        true
+        Ok(())
     }
 
     fn substitute(&self, _from: VarToken, _to: VarToken)
-            -> Option<Rc<Constraint>> {
+            -> PsResult<Rc<Constraint>> {
         unimplemented!();
     }
 }