From 685e5149d1d9aa3c27466d22f2b7012e99e3fc92 Mon Sep 17 00:00:00 2001 From: Tiffany Bennett Date: Thu, 25 Aug 2016 22:32:38 -0400 Subject: [PATCH] Use more deterministic data structures --- src/date.rs | 7 +++---- src/eval.rs | 30 ++++++++++++------------------ src/factorize.rs | 8 ++++---- src/number.rs | 27 ++++++++++++++++++++++++--- tests/query.rs | 21 +++++++++++++++++++-- 5 files changed, 62 insertions(+), 31 deletions(-) diff --git a/src/date.rs b/src/date.rs index a692fd9..0606a5e 100644 --- a/src/date.rs +++ b/src/date.rs @@ -6,7 +6,7 @@ use ast::{DatePattern, DateToken, show_datepattern}; use chrono::format::Parsed; use chrono::{Weekday, DateTime, UTC, FixedOffset, Duration, Date}; use eval::Context; -use number::Number; +use number::{Number, Dim}; use std::iter::Peekable; pub fn parse_date( @@ -225,7 +225,7 @@ pub fn to_duration(num: &Number) -> Result { use gmp::mpq::Mpq; use gmp::mpz::Mpz; - if num.1.len() != 1 || num.1.get(&"s".to_owned()) != Some(&1) { + if num.1.len() != 1 || num.1.get("s") != Some(&1) { return Err(format!("Expected seconds")) } let max = Mpq::ratio(&Mpz::from(i64::max_value() / 1000), &Mpz::one()); @@ -239,12 +239,11 @@ pub fn to_duration(num: &Number) -> Result { pub fn from_duration(duration: &Duration) -> Result { use gmp::mpq::Mpq; use gmp::mpz::Mpz; - use std::rc::Rc; let ns = try!(duration.num_nanoseconds() .ok_or(format!("Implementation error: Duration is out of range"))); let div = Mpz::from(1_000_000_000); - Ok(Number::new_unit(Mpq::ratio(&Mpz::from(ns), &div), Rc::new("s".to_owned()))) + Ok(Number::new_unit(Mpq::ratio(&Mpz::from(ns), &div), Dim::new("s"))) } pub fn now() -> DateTime { diff --git a/src/eval.rs b/src/eval.rs index 754c9e8..024dfb7 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -2,11 +2,10 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::collections::HashMap; -use std::collections::BTreeMap; +use std::collections::{HashMap, BTreeMap, BTreeSet}; use gmp::mpq::Mpq; use chrono::{DateTime, FixedOffset}; -use number::{Number, Unit}; +use number::{Number, Unit, Dim}; use date; use ast::{DatePattern, Expr, SuffixOp, Def, Defs, Query, Conversion}; use std::ops::{Add, Div, Mul, Neg, Sub}; @@ -22,10 +21,10 @@ pub enum Value { /// The evaluation context that contains unit definitions. #[derive(Debug)] pub struct Context { - pub dimensions: Vec>, + pub dimensions: BTreeSet, pub units: HashMap, pub aliases: HashMap, - pub reverse: HashMap, + pub reverse: BTreeMap, pub prefixes: Vec<(String, Number)>, pub definitions: HashMap, pub datepatterns: Vec>, @@ -161,10 +160,8 @@ impl Context { /// Given a unit name, returns its value if it exists. Supports SI /// prefixes, plurals, bare dimensions like length, and aliases. pub fn lookup(&self, name: &str) -> Option { - for k in &self.dimensions { - if name == &***k { - return Some(Number::one_unit(k.to_owned())) - } + if let Some(k) = self.dimensions.get(name) { + return Some(Number::one_unit(k.to_owned())) } if let Some(v) = self.units.get(name).cloned() { return Some(v) @@ -191,10 +188,8 @@ impl Context { /// Given a unit name, try to return a canonical name (expanding aliases and such) pub fn canonicalize(&self, name: &str) -> Option { - for k in &self.dimensions { - if name == &***k { - return Some((**k).clone()) - } + if let Some(k) = self.dimensions.get(name) { + return Some((*k.0).clone()) } if let Some(v) = self.definitions.get(name) { if let Expr::Unit(ref name) = *v { @@ -337,7 +332,7 @@ impl Context { match *expr { Expr::Unit(ref name) if name == "now" => Ok(Value::DateTime(date::now())), Expr::Unit(ref name) => self.lookup(name).ok_or(format!("Unknown unit {}", name)).map(Value::Number), - Expr::Quote(ref name) => Ok(Value::Number(Number::one_unit(Rc::new(name.clone())))), + Expr::Quote(ref name) => Ok(Value::Number(Number::one_unit(Dim::new(&**name)))), Expr::Const(ref num, ref frac, ref exp) => Number::from_parts( num, @@ -775,10 +770,10 @@ impl Context { /// Creates a new, empty context pub fn new() -> Context { Context { - dimensions: Vec::new(), + dimensions: BTreeSet::new(), units: HashMap::new(), aliases: HashMap::new(), - reverse: HashMap::new(), + reverse: BTreeMap::new(), prefixes: Vec::new(), definitions: HashMap::new(), datepatterns: Vec::new(), @@ -958,8 +953,7 @@ impl Context { }; match *def { Def::Dimension(ref dname) => { - let dname = Rc::new(dname.clone()); - self.dimensions.push(dname.clone()); + self.dimensions.insert(Dim::new(&**dname)); }, Def::Unit(ref expr) => match self.eval(expr) { Ok(Value::Number(v)) => { diff --git a/src/factorize.rs b/src/factorize.rs index 3ee95fa..5edcb9c 100644 --- a/src/factorize.rs +++ b/src/factorize.rs @@ -2,9 +2,9 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::collections::{BTreeMap, BinaryHeap, HashMap}; +use std::collections::{BTreeMap, BinaryHeap}; use std::rc::Rc; -use number::{Number, Unit}; +use number::{Number, Unit, Dim}; use std::cmp; use gmp::mpq::Mpq; @@ -23,7 +23,7 @@ impl cmp::Ord for Factors { } } -pub fn fast_decompose(value: &Number, aliases: &HashMap) -> Unit { +pub fn fast_decompose(value: &Number, aliases: &BTreeMap) -> Unit { let mut best = None; for (unit, name) in aliases.iter() { let num = Number(Mpq::one(), unit.clone()); @@ -39,7 +39,7 @@ pub fn fast_decompose(value: &Number, aliases: &HashMap) -> Unit { if let Some((name, unit, pow, score)) = best { if score < value.complexity_score() { let mut res = (value / &Number(Mpq::one(), unit.clone()).powi(pow)).unwrap().1; - res.insert(Rc::new(name.clone()), pow as i64); + res.insert(Dim::new(&**name), pow as i64); return res } } diff --git a/src/number.rs b/src/number.rs index baaa85f..8641bab 100644 --- a/src/number.rs +++ b/src/number.rs @@ -9,18 +9,39 @@ use eval::Show; use std::ops::{Add, Div, Mul, Neg, Sub}; use std::rc::Rc; use std::fmt; +use std::borrow::Borrow; /// Number type pub type Num = Mpq; -/// A simple alias to add semantic meaning for when we pass around dimension IDs. -pub type Dim = Rc; /// Alias for the primary representation of dimensionality. pub type Unit = BTreeMap; +/// A newtype for a string dimension ID, so that we can implement traits for it. +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +pub struct Dim(pub Rc); + /// The basic representation of a number with a unit. #[derive(Clone, PartialEq, Eq)] pub struct Number(pub Num, pub Unit); +impl Borrow for Dim { + fn borrow(&self) -> &str { + &**self.0 + } +} + +impl fmt::Display for Dim { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + self.0.fmt(fmt) + } +} + +impl Dim { + pub fn new(dim: &str) -> Dim { + Dim(Rc::new(dim.to_owned())) + } +} + fn one() -> Mpq { Mpq::one() } @@ -320,7 +341,7 @@ impl Show for Number { let e = value.1.iter().next().unwrap(); let ref n = *e.0; if *e.1 == 1 { - Some((**n).clone()) + Some((&*n.0).clone()) } else { Some(format!("{}^{}", n, e.1)) } diff --git a/tests/query.rs b/tests/query.rs index 4a6c641..44a01e6 100644 --- a/tests/query.rs +++ b/tests/query.rs @@ -16,9 +16,26 @@ fn test(input: &str, output: &str) { } #[test] -fn test_queries() { +fn test_definition() { test("watt", "Definition: watt = J / s = 1 watt (power; kg m^2 / s^3)"); +} + +#[test] +fn test_eval() { test("5 inch", "0.127 m (length)"); - test("5 inch -> cm", "12.7 cm (length)"); +} + +#[test] +fn test_convert() { + test("5 inch -> cm", "12.7 centim (length)"); +} + +#[test] +fn test_temp() { test("2 degC 2 -> degC", "277.15 °C (temperature)"); } + +#[test] +fn test_determinism() { + test("pascal m", "1 A tesla (spectral_irradiance_frequency)"); +}