From 04e8c554cc0a159a52936b98971a24f732ac212f Mon Sep 17 00:00:00 2001
From: artus40 <artus@landoftheunicorn.ovh>
Date: Thu, 14 Feb 2019 21:34:31 +0100
Subject: [PATCH] code cleanup

---
 planner/src/solver.rs | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/planner/src/solver.rs b/planner/src/solver.rs
index a6a3103..e5d4607 100644
--- a/planner/src/solver.rs
+++ b/planner/src/solver.rs
@@ -101,6 +101,12 @@ impl<V: fmt::Debug> fmt::Debug for Domain<V> {
 
 pub type Constraint<'a,V, K> = fn(&Variables<'a,V, K>) -> bool;
 
+
+/// Could be more efficient to just use fixed array of options as variables,
+/// using a helper to bind Keys to Index in this array.
+/// Domains could be a similar array of DomainValues.
+/// It makes sense with an array where indexing is O(1)
+
 pub struct Problem<'a, V, K> {
     /// The initial assignements map
     variables: Variables<'a, V, K>,
@@ -110,7 +116,7 @@ pub struct Problem<'a, V, K> {
     constraints: Vec<Constraint<'a,V,K>>,
 }
 
-impl<'a,V, K: Eq + Hash + Clone> Problem<'a, V, K> {
+impl<'a, V, K: Eq + Hash + Clone> Problem<'a, V, K> {
 
     pub fn build() -> ProblemBuilder<'a,V, K> {
         ProblemBuilder::new()
@@ -126,22 +132,37 @@ impl<'a,V, K: Eq + Hash + Clone> Problem<'a, V, K> {
     /// a Clear to ensure the variable is unset before when leaving the branch.
     fn _push_updates(&self) -> Option<Vec<Assignment<'a,V, K>>> {
         // TODO: should be able to inject a choosing strategy
-        if let Some((key,_)) = self.variables.iter().find(|(_, val)| val.is_none()) {
-            let domain_values = self.domains.get(key).expect("No domain for variable !");
+        if let Some(key) = self._next_assign() {
+            let domain_values = self.domains
+                                    .get(key)
+                                    .expect("No domain for variable !");
+            assert!(!domain_values.is_empty());
             // Push a clear assignment first, just before going up the stack.
             let mut updates = vec![Assignment::Clear(key.clone())];
-
-            if domain_values.is_empty() { panic!("No value in domain !"); }
             // TODO: should be able to filter domain values (inference, pertinence)
-            for value in domain_values.into_iter() {
-                updates.push(Assignment::Update(key.clone(), *value));
-            }
+            domain_values.iter().for_each(|value| {
+                updates.push(
+                    Assignment::Update(
+                        key.clone(),
+                        *value
+                    )
+                );
+            });
             Some(updates)
         } else { // End of assignements
             None
         }
     }
 
+    fn _next_assign(&self) -> Option<&K> {
+        self.variables
+            .iter()
+            .find_map(|(key, val)| {
+                    if val.is_none() { Some(key) }
+                    else { None }
+            })
+    }
+
     /// Checks that the current assignments doesn't violate any constraint
     fn _is_valid(&self) -> bool {
         for validator in self.constraints.iter() {