From 03b9822809b71fe302c83df2ddc832afd58777e0 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Sat, 28 Sep 2019 10:08:48 +0300 Subject: Refactor: use enum to express status --- src/lib.rs | 145 ++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 86 insertions(+), 59 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e5db651..414e83a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,11 +38,47 @@ use std::collections::HashMap; use std::error::Error; use textwrap::fill; +#[derive(Clone,Debug,PartialEq)] +pub enum Status { + Unknown, + Goal, + Finished, + Ready, + Next, + Blocked, +} + +impl Status { + fn from(text: &str) -> Option { + match text { + "" => Some(Status::Unknown), + "goal" => Some(Status::Goal), + "finished" => Some(Status::Finished), + "ready" => Some(Status::Ready), + "next" => Some(Status::Next), + "blocked" => Some(Status::Blocked), + _ => None, + } + } +} + +impl PartialEq for &Status { + fn eq(&self, other: &Status) -> bool { + **self == *other + } +} + +impl PartialEq<&Status> for Status { + fn eq(&self, other: &&Status) -> bool { + **other == *self + } +} + /// A step in a roadmap. #[derive(Clone, Debug, PartialEq)] pub struct Step { name: String, - status: String, + status: Status, label: String, depends: Vec, } @@ -52,7 +88,7 @@ impl Step { pub fn new(name: &str, label: &str) -> Step { Step { name: name.to_string(), - status: "".to_string(), + status: Status::Unknown, label: label.to_string(), depends: vec![], } @@ -69,13 +105,13 @@ impl Step { } /// Return the status of a step. - pub fn status<'a>(&'a self) -> &'a str { + pub fn status<'a>(&'a self) -> &Status { &self.status } /// Set the status of a step. - pub fn set_status(&mut self, status: &str) { - self.status = String::from(status); + pub fn set_status(&mut self, status: Status) { + self.status = status } /// Return vector of names of dependencies for a step. @@ -123,13 +159,13 @@ impl Roadmap { } // Convert a Value into a Step, if possible. - fn step_from_value(name: &str, value: &Value) -> Result { + fn step_from_value(name: &str, value: &Value) -> ParseResult { match value { Value::Mapping(_) => { let label = Roadmap::parse_label(&value); let mut step = Step::new(name, label); - let status = Roadmap::parse_status(&value); + let status = Roadmap::parse_status(&value)?; step.set_status(status); for depname in Roadmap::parse_depends(&value).iter() { @@ -138,7 +174,7 @@ impl Roadmap { Ok(step) } - _ => Err("step is not a mapping"), + _ => Err("step is not a mapping".to_string()), } } @@ -164,9 +200,15 @@ impl Roadmap { Roadmap::parse_string("label", map) } - // Get status string from a Mapping element, or empty string. - fn parse_status<'a>(map: &'a Value) -> &'a str { - Roadmap::parse_string("status", map) + // Get status string from a Mapping element. Default to unknown status. + fn parse_status<'a>(map: &'a Value) -> ParseResult { + let status_text = Roadmap::parse_string("status", map); + let status = Status::from(status_text); + if let Some(status) = status { + Ok(status) + } else { + Err(format!("unknown status: {:?}", status_text)) + } } // Get string value from a Mapping element, or empty string. @@ -180,21 +222,6 @@ impl Roadmap { // Validate that the parsed, constructed roadmap is valid. fn validate(&self) -> ParseResult<()> { - // Does every step has an acceptable status? - for step in self.steps.iter() { - let status = step.status(); - match status { - "" | "goal" | "ready" | "finished" | "next" | "blocked" => (), - _ => { - return Err(format!( - "step {:?} status {:?} is not allowed", - step.name(), - status - )) - } - } - } - // Is there exactly one goal? if self.count_goals() != 1 { return Err(format!("must have exactly one goal for roadmap")); @@ -274,11 +301,11 @@ impl Roadmap { let mut step = step.clone(); if self.is_unset(&step) { if self.is_goal(&step) { - step.set_status("goal"); + step.set_status(Status::Goal); } else if self.is_blocked(&step) { - step.set_status("blocked"); + step.set_status(Status::Blocked); } else if self.is_ready(&step) { - step.set_status("ready"); + step.set_status(Status::Ready); } } step @@ -293,7 +320,7 @@ impl Roadmap { // Is status unset? fn is_unset(&self, step: &Step) -> bool { - step.status() == "" + step.status() == Status::Unknown } // Should unset status be ready? In other words, if there are any @@ -301,7 +328,7 @@ impl Roadmap { fn is_ready(&self, step: &Step) -> bool { self.dep_statuses(step) .iter() - .all(|status| status == &"finished") + .all(|&status| status == Status::Finished) } // Should unset status be blocked? In other words, if there are @@ -309,24 +336,24 @@ impl Roadmap { fn is_blocked(&self, step: &Step) -> bool { self.dep_statuses(step) .iter() - .any(|status| status != &"finished") + .any(|&status| status != Status::Finished) } // Return vector of all statuses of all dependencies - fn dep_statuses<'a>(&'a self, step: &Step) -> Vec<&'a str> { + fn dep_statuses<'a>(&'a self, step: &Step) -> Vec<&Status> { step.dependencies() .map(|depname| { if let Some(step) = self.get_step(depname) { step.status() } else { - &"" + &Status::Unknown } }) .collect() } - // Should unset status be goal? In other words, does any other - // step depend on this one? + // Should status be goal? In other words, does any other step + // depend on this one? fn is_goal(&self, step: &Step) -> bool { let has_parent = self.steps.iter().any(|other| other.depends_on(step.name())); !has_parent @@ -366,36 +393,36 @@ impl Roadmap { fn get_status_color(step: &Step) -> &str { match step.status() { - "blocked" => "#f4bada", - "finished" => "#eeeeee", - "ready" => "#ffffff", - "next" => "#0cc00", - "goal" => "#00eeee", - _ => "unknownstatus", + Status::Blocked => "#f4bada", + Status::Finished => "#eeeeee", + Status::Ready => "#ffffff", + Status::Next => "#0cc00", + Status::Goal => "#00eeee", + Status::Unknown => "#rr0000", } } fn get_status_shape(step: &Step) -> &str { match step.status() { - "blocked" => "rectangle", - "finished" => "circle", - "ready" => "ellipse", - "next" => "ellipse", - "goal" => "diamond", - _ => "unknownshape", + Status::Blocked => "rectangle", + Status::Finished => "circle", + Status::Ready => "ellipse", + Status::Next => "ellipse", + Status::Goal => "diamond", + Status::Unknown => "house", } } } #[cfg(test)] mod tests { - use super::{Roadmap, Step}; + use super::{Roadmap, Step, Status}; #[test] fn new_step() { let step = Step::new("myname", "my label"); assert_eq!(step.name(), "myname"); - assert_eq!(step.status(), ""); + assert_eq!(step.status(), Status::Unknown); assert_eq!(step.label(), "my label"); assert_eq!(step.dependencies().count(), 0); } @@ -403,8 +430,8 @@ mod tests { #[test] fn set_status() { let mut step = Step::new("myname", "my label"); - step.set_status("next"); - assert_eq!(step.status(), "next"); + step.set_status(Status::Next); + assert_eq!(step.status(), Status::Next); } #[test] @@ -512,11 +539,11 @@ blocked: ) .unwrap(); r.set_missing_statuses(); - assert_eq!(r.get_step("goal").unwrap().status(), "goal"); - assert_eq!(r.get_step("finished").unwrap().status(), "finished"); - assert_eq!(r.get_step("ready").unwrap().status(), "ready"); - assert_eq!(r.get_step("next").unwrap().status(), "next"); - assert_eq!(r.get_step("blocked").unwrap().status(), "blocked"); + assert_eq!(r.get_step("goal").unwrap().status(), Status::Goal); + assert_eq!(r.get_step("finished").unwrap().status(), Status::Finished); + assert_eq!(r.get_step("ready").unwrap().status(), Status::Ready); + assert_eq!(r.get_step("next").unwrap().status(), Status::Next); + assert_eq!(r.get_step("blocked").unwrap().status(), Status::Blocked); } #[test] @@ -534,10 +561,10 @@ blocked: fn simple_dot() { let mut roadmap = Roadmap::new(); let mut first = Step::new("first", ""); - first.set_status("ready"); + first.set_status(Status::Ready); let mut second = Step::new("second", ""); second.add_dependency("first"); - second.set_status("goal"); + second.set_status(Status::Goal); roadmap.add_step(&first).unwrap(); roadmap.add_step(&second).unwrap(); assert_eq!( -- cgit v1.2.1