Repository: maciejhirsz/json-rust (rust)

Issue:
It's possible to remove the `iterators` module entirely.
We can define type aliases for `Members , MembersMut, Entries, EntriesMut`, for example:
```rust
pub type Members<'a> = slice::Iter<'a, JsonValue>;
```
And change methods to return `Option`:
```rust
pub fn members(&self) -> Option<Members>
```
It's a little inconvenient on a call site, but that way we can eliminate one level of indirection.
@maciejhirsz, If you're ok with this I can implement it.

Pull request description:
Remove the `iterators` module.
Closes #64 

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 5399a97..45d1bea 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "json"
-version = "0.8.8"
+version = "0.8.6"
 authors = ["Maciej Hirsz <maciej.hirsz@gmail.com>"]
 description = "JSON implementation in Rust"
 repository = "https://github.com/maciejhirsz/json-rust"
diff --git a/src/error.rs b/src/error.rs
index 48ea57d..5d1e733 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,9 +1,7 @@
-use std::error::Error;
-use std::fmt;
-use std::char;
+use std::{ char, error, fmt };
 
 #[derive(Debug, PartialEq)]
-pub enum JsonError {
+pub enum Error {
     UnexpectedCharacter {
         ch: char,
         line: usize,
@@ -16,15 +14,15 @@ pub enum JsonError {
     UndefinedField(String),
 }
 
-impl JsonError {
+impl Error {
     pub fn wrong_type(expected: &str) -> Self {
-        JsonError::WrongType(expected.into())
+        Error::WrongType(expected.into())
     }
 }
 
-impl fmt::Display for JsonError {
+impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use JsonError::*;
+        use Error::*;
 
         match *self {
             UnexpectedCharacter {
@@ -42,9 +40,9 @@ impl fmt::Display for JsonError {
     }
 }
 
-impl Error for JsonError {
+impl error::Error for Error {
     fn description(&self) -> &str {
-        use JsonError::*;
+        use Error::*;
         match *self {
             UnexpectedCharacter { .. } => "Unexpected character",
             UnexpectedEndOfJson        => "Unexpected end of JSON",
diff --git a/src/iterators.rs b/src/iterators.rs
deleted file mode 100644
index a798220..0000000
--- a/src/iterators.rs
+++ /dev/null
@@ -1,104 +0,0 @@
-use std::collections::btree_map;
-use std::slice;
-use std::iter::{ Iterator, DoubleEndedIterator };
-use JsonValue;
-
-pub enum Members<'a> {
-    Some(slice::Iter<'a, JsonValue>),
-    None
-}
-
-pub enum MembersMut<'a> {
-    Some(slice::IterMut<'a, JsonValue>),
-    None
-}
-
-pub enum Entries<'a> {
-    Some(btree_map::Iter<'a, String, JsonValue>),
-    None
-}
-
-pub enum EntriesMut<'a> {
-    Some(btree_map::IterMut<'a, String, JsonValue>),
-    None
-}
-
-impl<'a> Iterator for Members<'a> {
-    type Item = &'a JsonValue;
-
-    fn next(&mut self) -> Option<&'a JsonValue> {
-        match *self {
-            Members::Some(ref mut iter) => iter.next(),
-            Members::None               => None,
-        }
-    }
-}
-
-impl<'a> DoubleEndedIterator for Members<'a> {
-    fn next_back(&mut self) -> Option<&'a JsonValue> {
-        match *self {
-            Members::Some(ref mut iter) => iter.next_back(),
-            Members::None               => None,
-        }
-    }
-}
-
-impl<'a> Iterator for MembersMut<'a> {
-    type Item = &'a mut JsonValue;
-
-    fn next(&mut self) -> Option<&'a mut JsonValue> {
-        match *self {
-            MembersMut::Some(ref mut iter) => iter.next(),
-            MembersMut::None               => None,
-        }
-    }
-}
-
-impl<'a> DoubleEndedIterator for MembersMut<'a> {
-    fn next_back(&mut self) -> Option<&'a mut JsonValue> {
-        match *self {
-            MembersMut::Some(ref mut iter) => iter.next_back(),
-            MembersMut::None               => None,
-        }
-    }
-}
-
-impl<'a> Iterator for Entries<'a> {
-    type Item = (&'a String, &'a JsonValue);
-
-    fn next(&mut self) -> Option<(&'a String, &'a JsonValue)> {
-        match *self {
-            Entries::Some(ref mut iter) => iter.next(),
-            Entries::None               => None
-        }
-    }
-}
-
-impl<'a> DoubleEndedIterator for Entries<'a> {
-    fn next_back(&mut self) -> Option<(&'a String, &'a JsonValue)> {
-        match *self {
-            Entries::Some(ref mut iter) => iter.next_back(),
-            Entries::None               => None
-        }
-    }
-}
-
-impl<'a> Iterator for EntriesMut<'a> {
-    type Item = (&'a String, &'a mut JsonValue);
-
-    fn next(&mut self) -> Option<(&'a String, &'a mut JsonValue)> {
-        match *self {
-            EntriesMut::Some(ref mut iter) => iter.next(),
-            EntriesMut::None               => None
-        }
-    }
-}
-
-impl<'a> DoubleEndedIterator for EntriesMut<'a> {
-    fn next_back(&mut self) -> Option<(&'a String, &'a mut JsonValue)> {
-        match *self {
-            EntriesMut::Some(ref mut iter) => iter.next_back(),
-            EntriesMut::None               => None
-        }
-    }
-}
diff --git a/src/lib.rs b/src/lib.rs
index 16a04e7..00a6567 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -197,25 +197,42 @@
 //! # }
 //! ```
 
+use std::io::Write;
+use std::collections::{ BTreeMap, HashMap, btree_map };
+use std::{ fmt, result };
+use std::slice;
+
 mod codegen;
 mod parser;
 mod value;
 mod error;
-pub mod iterators;
 
-pub use error::JsonError;
+pub use error::Error;
 pub use value::JsonValue;
 pub use value::JsonValue::Null;
-pub type JsonResult<T> = Result<T, JsonError>;
+pub type Result<T> = result::Result<T, Error>;
+
+/// Iterator over members of `JsonValue::Array`.
+pub type Members<'a> = slice::Iter<'a, JsonValue>;
+
+/// Mutable iterator over members of `JsonValue::Array`.
+pub type MembersMut<'a> = slice::IterMut<'a, JsonValue>;
+
+/// Iterator over key value pairs of `JsonValue::Object`.
+pub type Entries<'a> = btree_map::Iter<'a, String, JsonValue>;
+
+/// Mutable iterator over key value pairs of `JsonValue::Object`.
+pub type EntriesMut<'a> = btree_map::IterMut<'a, String, JsonValue>;
+
+#[deprecated(since="0.9.0", note="use `json::Error` instead")]
+pub use Error as JsonError;
+
+#[deprecated(since="0.9.0", note="use `json::Result` instead")]
+pub use Result as JsonResult;
 
 pub use parser::parse;
 use codegen::{ Generator, PrettyGenerator, DumpGenerator, WriterGenerator };
 
-use std::io::Write;
-use std::collections::HashMap;
-use std::collections::BTreeMap;
-use std::fmt;
-
 pub type Array = Vec<JsonValue>;
 pub type Object = BTreeMap<String, JsonValue>;
 
diff --git a/src/parser.rs b/src/parser.rs
index 8ed60f4..d97dcce 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -1,6 +1,6 @@
 use std::{ str, char, f64 };
 use std::collections::BTreeMap;
-use { JsonValue, JsonError, JsonResult };
+use { JsonValue, Error, Result };
 
 const MAX_PRECISION: u64 = 576460752303423500;
 
@@ -19,7 +19,7 @@ struct Parser<'a> {
 macro_rules! expect_byte {
     ($parser:ident) => ({
         if $parser.is_eof() {
-            return Err(JsonError::UnexpectedEndOfJson);
+            return Err(Error::UnexpectedEndOfJson);
         }
 
         let ch = $parser.read_byte();
@@ -310,7 +310,7 @@ impl<'a> Parser<'a> {
         }
     }
 
-    fn unexpected_character<T: Sized>(&mut self, byte: u8) -> JsonResult<T> {
+    fn unexpected_character<T: Sized>(&mut self, byte: u8) -> Result<T> {
         let pos = self.source_position_from_index(self.index);
 
         let ch = if byte & 0x80 != 0 {
@@ -336,7 +336,7 @@ impl<'a> Parser<'a> {
 
             let slice = try!(
                 str::from_utf8(&buf[0..len])
-                .map_err(|_| JsonError::FailedUtf8Parsing)
+                .map_err(|_| Error::FailedUtf8Parsing)
             );
 
             slice.chars().next().unwrap()
@@ -346,14 +346,14 @@ impl<'a> Parser<'a> {
             unsafe { char::from_u32_unchecked(byte as u32) }
         };
 
-        Err(JsonError::UnexpectedCharacter {
+        Err(Error::UnexpectedCharacter {
             ch: ch,
             line: pos.line,
             column: pos.column,
         })
     }
 
-    fn read_hexdec_digit(&mut self) -> JsonResult<u32> {
+    fn read_hexdec_digit(&mut self) -> Result<u32> {
         let ch = expect_byte!(self);
         Ok(match ch {
             b'0' ... b'9' => (ch - b'0'),
@@ -363,7 +363,7 @@ impl<'a> Parser<'a> {
         } as u32)
     }
 
-    fn read_hexdec_codepoint(&mut self) -> JsonResult<u32> {
+    fn read_hexdec_codepoint(&mut self) -> Result<u32> {
         Ok(
             try!(self.read_hexdec_digit()) << 12 |
             try!(self.read_hexdec_digit()) << 8  |
@@ -372,7 +372,7 @@ impl<'a> Parser<'a> {
         )
     }
 
-    fn read_codepoint(&mut self, buffer: &mut Vec<u8>) -> JsonResult<()> {
+    fn read_codepoint(&mut self, buffer: &mut Vec<u8>) -> Result<()> {
         let mut codepoint = try!(self.read_hexdec_codepoint());
 
         match codepoint {
@@ -388,11 +388,11 @@ impl<'a> Parser<'a> {
                 if let 0xDC00 ... 0xDFFF = lower {
                     codepoint = (codepoint | lower - 0xDC00) + 0x010000;
                 } else {
-                    return Err(JsonError::FailedUtf8Parsing)
+                    return Err(Error::FailedUtf8Parsing)
                 }
             },
             0xE000 ... 0xFFFF => {},
-            _ => return Err(JsonError::FailedUtf8Parsing)
+            _ => return Err(Error::FailedUtf8Parsing)
         }
 
         match codepoint {
@@ -412,13 +412,13 @@ impl<'a> Parser<'a> {
                 (((codepoint >> 6)  as u8) & 0x3F) | 0x80,
                 ((codepoint         as u8) & 0x3F) | 0x80
             ]),
-            _ => return Err(JsonError::FailedUtf8Parsing)
+            _ => return Err(Error::FailedUtf8Parsing)
         }
 
         Ok(())
     }
 
-    fn read_complex_string(&mut self, start: usize) -> JsonResult<String> {
+    fn read_complex_string(&mut self, start: usize) -> Result<String> {
         let mut buffer = Vec::new();
         let mut ch = b'\\';
 
@@ -462,7 +462,7 @@ impl<'a> Parser<'a> {
         Ok(unsafe { String::from_utf8_unchecked(buffer) })
     }
 
-    fn read_big_number(&mut self, num: u64) -> JsonResult<f64> {
+    fn read_big_number(&mut self, num: u64) -> Result<f64> {
         // Attempt to continue reading digits that would overflow
         // u64 into freshly converted f64
 
@@ -483,7 +483,7 @@ impl<'a> Parser<'a> {
         self.read_number_with_fraction(num, e)
     }
 
-    fn read_number_with_fraction(&mut self, mut num: u64, mut e: i32) -> JsonResult<f64> {
+    fn read_number_with_fraction(&mut self, mut num: u64, mut e: i32) -> Result<f64> {
         if self.is_eof() {
             return Ok(make_float(num, e));
         }
@@ -542,7 +542,7 @@ impl<'a> Parser<'a> {
         Ok(make_float(num, e))
     }
 
-    fn read_object(&mut self) -> JsonResult<BTreeMap<String, JsonValue>> {
+    fn read_object(&mut self) -> Result<BTreeMap<String, JsonValue>> {
         let mut object = BTreeMap::new();
 
         let key = expect!{ self,
@@ -571,7 +571,7 @@ impl<'a> Parser<'a> {
         Ok(object)
     }
 
-    fn read_array(&mut self) -> JsonResult<Vec<JsonValue>> {
+    fn read_array(&mut self) -> Result<Vec<JsonValue>> {
         let first = expect_value!{ self, b']' => return Ok(Vec::new()) };
 
         let mut array = Vec::with_capacity(20);
@@ -590,7 +590,7 @@ impl<'a> Parser<'a> {
         Ok(array)
     }
 
-    fn ensure_end(&mut self) -> JsonResult<()> {
+    fn ensure_end(&mut self) -> Result<()> {
         while !self.is_eof() {
             match self.read_byte() {
                 9 ... 13 | 32 => self.bump(),
@@ -604,12 +604,12 @@ impl<'a> Parser<'a> {
         Ok(())
     }
 
-    fn value(&mut self) -> JsonResult<JsonValue> {
+    fn value(&mut self) -> Result<JsonValue> {
         Ok(expect_value!(self))
     }
 }
 
-pub fn parse(source: &str) -> JsonResult<JsonValue> {
+pub fn parse(source: &str) -> Result<JsonValue> {
     let mut parser = Parser::new(source);
 
     let value = try!(parser.value());
diff --git a/src/value.rs b/src/value.rs
index b7ba7a4..48044da 100644
--- a/src/value.rs
+++ b/src/value.rs
@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 use std::ops::{ Index, IndexMut, Deref };
-use iterators::{ Members, MembersMut, Entries, EntriesMut };
-use { JsonResult, JsonError };
+use { Members, MembersMut, Entries, EntriesMut };
+use { Result, Error };
 use std::{ mem, usize, u8, u16, u32, u64, isize, i8, i16, i32, i64, f32 };
 
 macro_rules! f64_to_unsinged {
@@ -177,8 +177,6 @@ impl JsonValue {
 
     /// Take over the ownership of the value, leaving `Null` in it's place.
     ///
-    /// ## Example
-    ///
     /// ```
     /// # #[macro_use] extern crate json;
     /// # fn main() {
@@ -195,16 +193,19 @@ impl JsonValue {
     /// # }
     /// ```
     pub fn take(&mut self) -> JsonValue {
-        mem::replace(self, JsonValue::Null)
+        let mut placeholder = JsonValue::Null;
+
+        mem::swap(self, &mut placeholder);
+
+        placeholder
     }
 
-    /// Checks that self is a string, returns an owned Rust `String, leaving
-    /// `Null` in it's place.
-    ///
-    /// This is the cheapest way to obtain an owned `String` from JSON, as no
-    /// extra heap allocation is performend.
+    /// Checks that self is a string, and if so takes the ownership of it,
+    /// leaving a `Null` in it's place, and returning an owned Rust `String`.
     ///
-    /// ## Example
+    /// While this modifies the original structure, it is cheaper than creating
+    /// an owned `String` from the slice returned from `as_str`, since no cloning
+    /// or heap allocation is involved.
     ///
     /// ```
     /// # #[macro_use] extern crate json;
@@ -234,14 +235,14 @@ impl JsonValue {
 
     /// Works on `JsonValue::Array` - pushes a new value to the array.
     #[must_use]
-    pub fn push<T>(&mut self, value: T) -> JsonResult<()>
+    pub fn push<T>(&mut self, value: T) -> Result<()>
     where T: Into<JsonValue> {
         match *self {
             JsonValue::Array(ref mut vec) => {
                 vec.push(value.into());
                 Ok(())
             },
-            _ => Err(JsonError::wrong_type("Array"))
+            _ => Err(Error::wrong_type("Array"))
         }
     }
 
@@ -281,43 +282,43 @@ impl JsonValue {
     }
 
     /// Works on `JsonValue::Array` - returns an iterator over members.
-    pub fn members(&self) -> Members {
+    pub fn members(&self) -> Option<Members> {
         match *self {
             JsonValue::Array(ref vec) => {
-                Members::Some(vec.iter())
+                Some(vec.iter())
             },
-            _ => Members::None
+            _ => None
         }
     }
 
     /// Works on `JsonValue::Array` - returns a mutable iterator over members.
-    pub fn members_mut(&mut self) -> MembersMut {
+    pub fn members_mut(&mut self) -> Option<MembersMut> {
         match *self {
             JsonValue::Array(ref mut vec) => {
-                MembersMut::Some(vec.iter_mut())
+                Some(vec.iter_mut())
             },
-            _ => MembersMut::None
+            _ => None
         }
     }
 
     /// Works on `JsonValue::Object` - returns an iterator over key value pairs.
-    pub fn entries(&self) -> Entries {
+    pub fn entries(&self) -> Option<Entries> {
         match *self {
             JsonValue::Object(ref btree) => {
-                Entries::Some(btree.iter())
+                Some(btree.iter())
             },
-            _ => Entries::None
+            _ => None
         }
     }
 
     /// Works on `JsonValue::Object` - returns a mutable iterator over
     /// key value pairs.
-    pub fn entries_mut(&mut self) -> EntriesMut {
+    pub fn entries_mut(&mut self) -> Option<EntriesMut> {
         match *self {
             JsonValue::Object(ref mut btree) => {
-                EntriesMut::Some(btree.iter_mut())
+                Some(btree.iter_mut())
             },
-            _ => EntriesMut::None
+            _ => None
         }
     }
 
@@ -347,8 +348,6 @@ impl JsonValue {
 
 /// Implements indexing by `usize` to easily access array members:
 ///
-/// ## Example
-///
 /// ```
 /// # use json::JsonValue;
 /// let mut array = JsonValue::new_array();
@@ -370,8 +369,6 @@ impl Index<usize> for JsonValue {
 
 /// Implements mutable indexing by `usize` to easily modify array members:
 ///
-/// ## Example
-///
 /// ```
 /// # #[macro_use]
 /// # extern crate json;
@@ -408,8 +405,6 @@ impl IndexMut<usize> for JsonValue {
 
 /// Implements indexing by `&str` to easily access object members:
 ///
-/// ## Example
-///
 /// ```
 /// # #[macro_use]
 /// # extern crate json;
@@ -454,8 +449,6 @@ impl<'a> Index<&'a String> for JsonValue {
 
 /// Implements mutable indexing by `&str` to easily modify object members:
 ///
-/// ## Example
-///
 /// ```
 /// # #[macro_use]
 /// # extern crate json;
@@ -496,3 +489,17 @@ impl<'a> IndexMut<&'a String> for JsonValue {
         self.index_mut(index.deref())
     }
 }
+
+impl IntoIterator for JsonValue {
+    type Item = JsonValue;
+    type IntoIter = ::std::vec::IntoIter<JsonValue>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        match self {
+            JsonValue::Array(vec) => {
+                vec.into_iter()
+            },
+            _ => Vec::new().into_iter()
+        }
+    }
+}


Test patch:
diff --git a/tests/lib.rs b/tests/lib.rs
index 0be2112..b1bcabd 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -615,37 +615,22 @@ mod unit {
     fn array_members() {
         let data = array![1, "foo"];
 
-        for member in data.members() {
+        for member in data.members().unwrap() {
             assert!(!member.is_null());
         }
 
-        let mut members = data.members();
+        let mut members = data.members().unwrap();
 
         assert_eq!(members.next().unwrap(), 1);
         assert_eq!(members.next().unwrap(), "foo");
         assert!(members.next().is_none());
     }
 
-    #[test]
-    fn array_members_rev() {
-        let data = array![1, "foo"];
-
-        for member in data.members() {
-            assert!(!member.is_null());
-        }
-
-        let mut members = data.members().rev();
-
-        assert_eq!(members.next().unwrap(), "foo");
-        assert_eq!(members.next().unwrap(), 1);
-        assert!(members.next().is_none());
-    }
-
     #[test]
     fn array_members_mut() {
         let mut data = array![Null, Null];
 
-        for member in data.members_mut() {
+        for member in data.members_mut().unwrap() {
             assert!(member.is_null());
             *member = 100.into();
         }
@@ -653,20 +638,6 @@ mod unit {
         assert_eq!(data, array![100, 100]);
     }
 
-    #[test]
-    fn array_members_mut_rev() {
-        let mut data = array![Null, Null];
-        let mut item = 100;
-
-        for member in data.members_mut().rev() {
-            assert!(member.is_null());
-            *member = item.into();
-            item += 1;
-        }
-
-        assert_eq!(data, array![item - 1, item - 2]);
-    }
-
     #[test]
     fn object_len() {
         let data = object!{
@@ -695,11 +666,11 @@ mod unit {
             "b" => "foo"
         };
 
-        for (_, value) in data.entries() {
+        for (_, value) in data.entries().unwrap() {
             assert!(!value.is_null());
         }
 
-        let mut entries = data.entries();
+        let mut entries = data.entries().unwrap();
 
         let (key, value) = entries.next().unwrap();
         assert_eq!(key, "a");
@@ -712,30 +683,6 @@ mod unit {
         assert!(entries.next().is_none());
     }
 
-    #[test]
-    fn object_entries_rev() {
-        let data = object!{
-            "a" => 1,
-            "b" => "foo"
-        };
-
-        for (_, value) in data.entries().rev() {
-            assert!(!value.is_null());
-        }
-
-        let mut entries = data.entries().rev();
-
-        let (key, value) = entries.next().unwrap();
-        assert_eq!(key, "b");
-        assert_eq!(value, "foo");
-
-        let (key, value) = entries.next().unwrap();
-        assert_eq!(key, "a");
-        assert_eq!(value, 1);
-
-        assert!(entries.next().is_none());
-    }
-
     #[test]
     fn object_entries_mut() {
         let mut data = object!{
@@ -743,7 +690,7 @@ mod unit {
             "b" => Null
         };
 
-        for (_, value) in data.entries_mut() {
+        for (_, value) in data.entries_mut().unwrap() {
             assert!(value.is_null());
             *value = 100.into();
         }
@@ -754,26 +701,6 @@ mod unit {
         });
     }
 
-    #[test]
-    fn object_entries_mut_rev() {
-        let mut data = object!{
-            "a" => Null,
-            "b" => Null
-        };
-        let mut item = 100;
-
-        for (_, value) in data.entries_mut().rev() {
-            assert!(value.is_null());
-            *value = item.into();
-            item += 1;
-        }
-
-        assert_eq!(data, object!{
-            "a" => item - 1,
-            "b" => item - 2
-        });
-    }
-
     #[test]
     fn object_dump_minified() {
         let object = object!{
@@ -977,6 +904,20 @@ mod unit {
 
         assert_eq!(String::from_utf8(buf).unwrap(), r#"{"foo":["bar",100,true]}"#);
     }
+
+    #[test]
+    fn into_iter() {
+        let mut iter = array!["foo", true, 42].into_iter();
+        assert_eq!(iter.next(), Some("foo".into()));
+        assert_eq!(iter.next(), Some(true.into()));
+        assert_eq!(iter.next(), Some(42.into()));
+
+        let mut empty_iter = object!{"foo" => "bar"}.into_iter();
+        assert_eq!(empty_iter.next(), None);
+
+        let mut empty_iter = JsonValue::Null.into_iter();
+        assert_eq!(empty_iter.next(), None);
+    }
 }
 
 mod json_checker_fail {