From a91331d10f2b266200508a1184a3277d333751bb Mon Sep 17 00:00:00 2001
From: Taylor Bockman <angrygoats@protonmail.com>
Date: Sun, 17 Dec 2017 11:00:14 -0800
Subject: [PATCH] Generating option string finally done, now on to sending
 it...

---
 CONTRIBUTING.md |  4 ++++
 src/options.rs  | 52 ++++++++++++++++++++++++++++------------------------
 tests/lib.rs    |  2 +-
 3 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index f686ead..0a1dccd 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -6,3 +6,7 @@ TODO
 # Styleguide
 
 We try to make use of the styleguide found [here](https://github.com/rust-lang-nursery/fmt-rfcs/blob/master/guide/guide.md).
+
+## Character Limit
+
+We try to make each line less than or equal to 120 charcters.
diff --git a/src/options.rs b/src/options.rs
index 4e2d60f..e712afe 100644
--- a/src/options.rs
+++ b/src/options.rs
@@ -90,39 +90,43 @@ impl EngineOption {
     EngineOption { name, option_type, option_data, }
   }
 
-  fn engine_option_data_type_string(&t: EngineOptionDataType) -> String {
-    match t {
-      EngineOptionDataType::DefaultVal => String::new("default"),
-      EngineOptionDataType::Min => String::new("min"),
-      EngineOptionDataType::Max => String::new("max"),
-      EngineOptionDataType::Var => String::new("var"),
-    }
-  }
-
-  fn engine_option_data_string(&d: EngineOptionData) -> String {
-    match eod {
-      EngineOptionData::Int(v) => v.to_string(),
-      EngineOptionData::Float(v) => v.to_string(),
-      EngineOptionData::Text(ref v) => v.clone(),
+  fn engine_option_data_string(d: &EngineOptionData) -> String {
+    match d {
+      &EngineOptionData::Int(v) => v.to_string(),
+      &EngineOptionData::Float(v) => v.to_string(),
+      &EngineOptionData::Text(ref v) => v.clone(),
     }
   }
 
   pub fn option_string(&self) -> String {
     let mut option_data_string: String = String::new();
 
-    // FIXME: Hash maps are iterated out of order (obviously). In order to guarantee order in the response it'll be
-    //        Easier to test if the key exists (since there are only 3 of them ever), if it does, get the value with
-    //        HashMap::Get, which returns a reference.
-    //
-    //        Test for the key with HashMap::contains_key. You'll have to match for each instance using the private
-    //        functions above. You will _still_ need to build a string like you did before.
-    for (dt, eod) in self.option_data.iter() {
-      let _: (&EngineOptionDataType, &EngineOptionData) = (dt, eod);
+    // NOTE: The user is left to understand what option takes which of these data type values. There's some
+    //       work that can be done later switching based on the option type, but for now it's reasonable to
+    //       expect the user understands the standard well enough to know which options take what kind
+    //       of settings.
+    if self.option_data.contains_key(&EngineOptionDataType::DefaultVal) {
+      let data = self.option_data.get(&EngineOptionDataType::DefaultVal);
+      option_data_string.push_str(&format!(" {} {}", "default",
+                                           EngineOption::engine_option_data_string(&data.unwrap())));
+    }
+
+    if self.option_data.contains_key(&EngineOptionDataType::Min) {
+      let data = self.option_data.get(&EngineOptionDataType::Min);
+      option_data_string.push_str(&format!(" {} {}", "min", EngineOption::engine_option_data_string(&data.unwrap())));
+    }
 
+    if self.option_data.contains_key(&EngineOptionDataType::Max) {
+      let data = self.option_data.get(&EngineOptionDataType::Max);
+      option_data_string.push_str(&format!(" {} {}", "max", EngineOption::engine_option_data_string(&data.unwrap())));
+    }
 
-      option_data_string.push_str(&format!("{} {} ", dts, res));
+    if self.option_data.contains_key(&EngineOptionDataType::Var) {
+      let data = self.option_data.get(&EngineOptionDataType::Var);
+      option_data_string.push_str(&format!(" {} {}", "var", EngineOption::engine_option_data_string(&data.unwrap())));
     }
 
+
     let ots = match self.option_type {
       EngineOptionType::Check => "check",
       EngineOptionType::Spin => "spin",
@@ -131,7 +135,7 @@ impl EngineOption {
       EngineOptionType::TypeString => "string",
     };
 
-    format!("{} {} {} {} {} {}\n", commands::OPTION, commands::OPTIONNAME, self.name,
+    format!("{} {} {} {} {}{}\n", commands::OPTION, commands::OPTIONNAME, self.name,
             commands::TYPE, ots, option_data_string)
   }
 }
diff --git a/tests/lib.rs b/tests/lib.rs
index c8490db..ca7169f 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -92,7 +92,7 @@ fn engine_option_string() {
       option_data: option_data,
     };
 
-    let expected = "option name hash type spin default 1 min 1 max 128\n";
+    let expected = "option name Hash type spin default 1 min 1 max 128\n";
     assert_eq!(o.option_string(), expected);
 }