From 2ecda0a2bd2cefa1beeef92aa24a758cc5a0f2d0 Mon Sep 17 00:00:00 2001
From: Victor Porof <vporof@mozilla.com>
Date: Thu, 2 Feb 2017 15:38:38 +0100
Subject: [PATCH] Avoid needless reborrows and simplify `Ord` implementation
 for edn::Value

---
 edn/src/types.rs | 63 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/edn/src/types.rs b/edn/src/types.rs
index aa1829d4..d06b1dde 100644
--- a/edn/src/types.rs
+++ b/edn/src/types.rs
@@ -136,6 +136,14 @@ impl Value {
     pub fn from_bigint(src: &str) -> Option<Value> {
         src.parse::<BigInt>().map(Value::BigInteger).ok()
     }
+
+    pub fn from_symbol<'a, T: Into<Option<&'a str>>>(namespace: T, name: &str) -> Value {
+        to_symbol(namespace, name)
+    }
+
+    pub fn from_keyword<'a, T: Into<Option<&'a str>>>(namespace: T, name: &str) -> Value {
+        to_keyword(namespace, name)
+    }
 }
 
 impl From<f64> for Value {
@@ -150,28 +158,24 @@ impl PartialOrd for Value {
     }
 }
 
-// TODO: Check we follow the equality rules at the bottom of https://github.com/edn-format/edn
 impl Ord for Value {
     fn cmp(&self, other: &Value) -> Ordering {
-
-        let ord_order = to_ord(self).cmp(&to_ord(other));
-        match *self {
-            Nil             => match *other { Nil             => Ordering::Equal, _ => ord_order },
-            Boolean(bs)     => match *other { Boolean(bo)     => bo.cmp(&bs), _ => ord_order },
-            BigInteger(ref bs) => match *other { BigInteger(ref bo) => bo.cmp(&bs), _ => ord_order },
-            Integer(is)     => match *other { Integer(io)     => io.cmp(&is), _ => ord_order },
-            Float(ref fs)   => match *other { Float(ref fo)   => fo.cmp(&fs), _ => ord_order },
-            Text(ref ts)    => match *other { Text(ref to)    => to.cmp(&ts), _ => ord_order },
-            PlainSymbol(ref ss)  => match *other { PlainSymbol(ref so)  => so.cmp(&ss), _ => ord_order },
-            NamespacedSymbol(ref ss)
-                => match *other { NamespacedSymbol(ref so)    => so.cmp(&ss), _ => ord_order },
-            Keyword(ref ks) => match *other { Keyword(ref ko) => ko.cmp(&ks), _ => ord_order },
-            NamespacedKeyword(ref ks)
-                => match *other { NamespacedKeyword(ref ko)   => ko.cmp(&ks), _ => ord_order },
-            Vector(ref vs)  => match *other { Vector(ref vo)  => vo.cmp(&vs), _ => ord_order },
-            List(ref ls)    => match *other { List(ref lo)    => lo.cmp(&ls), _ => ord_order },
-            Set(ref ss)     => match *other { Set(ref so)     => so.cmp(&ss), _ => ord_order },
-            Map(ref ms)     => match *other { Map(ref mo)     => mo.cmp(&ms), _ => ord_order },
+        match (self, other) {
+            (&Nil, &Nil) => Ordering::Equal,
+            (&Boolean(a), &Boolean(b)) => b.cmp(&a),
+            (&Integer(a), &Integer(b)) => b.cmp(&a),
+            (&BigInteger(ref a), &BigInteger(ref b)) => b.cmp(a),
+            (&Float(ref a), &Float(ref b)) => b.cmp(a),
+            (&Text(ref a), &Text(ref b)) => b.cmp(a),
+            (&PlainSymbol(ref a), &PlainSymbol(ref b)) => b.cmp(a),
+            (&NamespacedSymbol(ref a), &NamespacedSymbol(ref b)) => b.cmp(a),
+            (&Keyword(ref a), &Keyword(ref b)) => b.cmp(a),
+            (&NamespacedKeyword(ref a), &NamespacedKeyword(ref b)) => b.cmp(a),
+            (&Vector(ref a), &Vector(ref b)) => b.cmp(a),
+            (&List(ref a), &List(ref b)) => b.cmp(a),
+            (&Set(ref a), &Set(ref b)) => b.cmp(a),
+            (&Map(ref a), &Map(ref b)) => b.cmp(a),
+            _ => to_ord(self).cmp(&to_ord(other))
         }
     }
 }
@@ -271,4 +275,23 @@ mod test {
             Value::NamespacedSymbol(symbols::NamespacedSymbol::new("nine", "ten")),
             Value::Boolean(true))).to_string());
     }
+
+    #[test]
+    fn test_ord() {
+        // TODO: Check we follow the equality rules at the bottom of https://github.com/edn-format/edn
+        assert_eq!(Value::Nil.cmp(&Value::Nil), Ordering::Equal);
+        assert_eq!(Value::Boolean(false).cmp(&Value::Boolean(true)), Ordering::Greater);
+        assert_eq!(Value::Integer(1).cmp(&Value::Integer(2)), Ordering::Greater);
+        assert_eq!(Value::from_bigint("1").cmp(&Value::from_bigint("2")), Ordering::Greater);
+        assert_eq!(Value::from(1f64).cmp(&Value::from(2f64)), Ordering::Greater);
+        assert_eq!(Value::Text("1".to_string()).cmp(&Value::Text("2".to_string())), Ordering::Greater);
+        assert_eq!(Value::from_symbol("a", "b").cmp(&Value::from_symbol("c", "d")), Ordering::Greater);
+        assert_eq!(Value::from_symbol(None, "a").cmp(&Value::from_symbol(None, "b")), Ordering::Greater);
+        assert_eq!(Value::from_keyword(":a", ":b").cmp(&Value::from_keyword(":c", ":d")), Ordering::Greater);
+        assert_eq!(Value::from_keyword(None, ":a").cmp(&Value::from_keyword(None, ":b")), Ordering::Greater);
+        assert_eq!(Value::Vector(vec![]).cmp(&Value::Vector(vec![])), Ordering::Equal);
+        assert_eq!(Value::List(LinkedList::new()).cmp(&Value::List(LinkedList::new())), Ordering::Equal);
+        assert_eq!(Value::Set(BTreeSet::new()).cmp(&Value::Set(BTreeSet::new())), Ordering::Equal);
+        assert_eq!(Value::Map(BTreeMap::new()).cmp(&Value::Map(BTreeMap::new())), Ordering::Equal);
+    }
 }
\ No newline at end of file