Remove just_seeked from DBRawIterator

This as added to make while-loops easy:

    while iter.next() {
        ...
    }

But this involves adding a layer of custom logic on top of the RocksDB
API. This is probably a bad idea given that this API is meant to closely
match the underlying iterator API in RocksDB, so this commit changes
that.

The new way to iterate is as follows:

while iter.valid() {
    ...

    iter.next();
}

src/db.rs

@@ -116,26 +116,28 @@ pub struct Snapshot<'a> {
 ///
 /// // Forwards iteration
 /// iter.seek_to_first();
-/// while iter.next() {
+/// while iter.valid() {
 ///     println!("Saw {:?} {:?}", iter.key(), iter.value());
+///     iter.next();
 /// }
 ///
 /// // Reverse iteration
 /// iter.seek_to_last();
-/// while iter.prev() {
+/// while iter.valid() {
 ///     println!("Saw {:?} {:?}", iter.key(), iter.value());
+///     iter.prev();
 /// }
 ///
 /// // Seeking
 /// iter.seek(b"my key");
-/// while iter.next() {
+/// while iter.valid() {
 ///     println!("Saw {:?} {:?}", iter.key(), iter.value());
+///     iter.next();
 /// }
 ///
 /// ```
 pub struct DBRawIterator {
     inner: *mut ffi::rocksdb_iterator_t,
-    just_seeked: bool,
 }
 
 
@@ -169,6 +171,7 @@ pub struct DBRawIterator {
 pub struct DBIterator {
     raw: DBRawIterator,
     direction: Direction,
+    just_seeked: bool,
 }
 
 pub enum Direction {
@@ -187,14 +190,9 @@ pub enum IteratorMode<'a> {
 impl DBRawIterator {
     fn new(db: &DB, readopts: &ReadOptions) -> DBRawIterator {
         unsafe {
-            let iterator = ffi::rocksdb_create_iterator(db.inner, readopts.inner);
+            DBRawIterator {
-
+                inner: ffi::rocksdb_create_iterator(db.inner, readopts.inner),
-            let mut rv = DBRawIterator {
+            }
-                inner: iterator,
-                just_seeked: false,
-            };
-            rv.seek_to_first();
-            rv
         }
     }
 
@@ -203,14 +201,9 @@ impl DBRawIterator {
               readopts: &ReadOptions)
               -> Result<DBRawIterator, Error> {
         unsafe {
-            let iterator = ffi::rocksdb_create_iterator_cf(db.inner, readopts.inner, cf_handle.inner);
+            Ok(DBRawIterator {
-
+                inner: ffi::rocksdb_create_iterator_cf(db.inner, readopts.inner, cf_handle.inner),
-            let mut rv = DBRawIterator {
+            })
-                inner: iterator,
-                just_seeked: false,
-            };
-            rv.seek_to_first();
-            Ok(rv)
         }
     }
 
@@ -221,8 +214,6 @@ impl DBRawIterator {
 
     /// Seeks to the first key in the database.
     ///
-    /// You must call ``.next()`` before reading the key.
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -235,16 +226,17 @@ impl DBRawIterator {
     ///
     /// iter.seek_to_first();
     ///
-    /// while iter.next() {
+    /// while iter.valid() {
     ///    println!("{:?} {:?}", iter.key(), iter.value());
+    ///
+    ///    iter.next();
     /// }
     ///
     /// // Read just the first key
     ///
     /// iter.seek_to_first();
     ///
-    /// let is_valid = iter.next();
+    /// if iter.valid() {
-    /// if is_valid {
     ///    println!("{:?} {:?}", iter.key(), iter.value());
     /// } else {
     ///    // There are no keys in the database
@@ -252,13 +244,10 @@ impl DBRawIterator {
     /// ```
     pub fn seek_to_first(&mut self) {
         unsafe { ffi::rocksdb_iter_seek_to_first(self.inner); }
-        self.just_seeked = true;
     }
 
     /// Seeks to the last key in the database.
     ///
-    /// You must call ``.prev()`` before reading the key.
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -271,16 +260,17 @@ impl DBRawIterator {
     ///
     /// iter.seek_to_last();
     ///
-    /// while iter.prev() {
+    /// while iter.valid() {
-    ///    println!("{:?} {:?}", iter.key(), iter.value());;
+    ///    println!("{:?} {:?}", iter.key(), iter.value());
+    ///
+    ///    iter.prev();
     /// }
     ///
     /// // Read just the last key
     ///
     /// iter.seek_to_last();
     ///
-    /// let is_valid = iter.prev();
+    /// if iter.valid() {
-    /// if is_valid {
     ///    println!("{:?} {:?}", iter.key(), iter.value());
     /// } else {
     ///    // There are no keys in the database
@@ -288,7 +278,6 @@ impl DBRawIterator {
     /// ```
     pub fn seek_to_last(&mut self) {
         unsafe { ffi::rocksdb_iter_seek_to_last(self.inner); }
-        self.just_seeked = true;
     }
 
     /// Seeks to the specified key or the first key that lexicographically follows it.
@@ -296,8 +285,6 @@ impl DBRawIterator {
     /// This method will attempt to seek to the specified key. If that key does not exist, it will
     /// find and seek to the key that lexicographically follows it instead.
     ///
-    /// Like the other seek methods, you must call ``.next()`` or ``.prev()`` before reading a key.
-    ///
     /// # Examples
     ///
     /// ```rust
@@ -310,8 +297,7 @@ impl DBRawIterator {
     ///
     /// iter.seek(b"a");
     ///
-    /// let is_valid = iter.next();
+    /// if iter.valid() {
-    /// if is_valid {
     ///    println!("{:?} {:?}", iter.key(), iter.value());
     /// } else {
     ///    // There are no keys in the database
@@ -319,7 +305,6 @@ impl DBRawIterator {
     /// ```
     pub fn seek(&mut self, key: &[u8]) {
         unsafe { ffi::rocksdb_iter_seek(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); }
-        self.just_seeked = true;
     }
 
 /*
@@ -331,7 +316,6 @@ impl DBRawIterator {
     /// The difference with ``.seek()`` is that if the specified key do not exist, this method will
     /// seek to key that lexicographically precedes it instead.
     ///
-    /// Like the other seek methods, you must call ``.next()`` or ``.prev()`` before reading a key.
     /// # Examples
     ///
     /// ```rust
@@ -344,51 +328,30 @@ impl DBRawIterator {
     ///
     /// iter.seek_for_prev(b"b");
     ///
-    /// let is_valid = iter.prev();
+    /// if iter.valid() {
-    /// if is_valid {
     ///    println!("{:?} {:?}", iter.key(), iter.value());
     /// } else {
     ///    // There are no keys in the database
     /// }
     pub fn seek_for_prev(&mut self, key: &[u8]) {
         unsafe { ffi::rocksdb_iter_seek_for_prev(self.inner, key.as_ptr() as *const c_char, key.len() as size_t); }
-        self.just_seeked = true;
     }
 */
 
     /// Seeks to the next key.
     ///
     /// Returns true if the iterator is valid after this operation.
-    pub fn next(&mut self) -> bool {
+    pub fn next(&mut self) {
-        // Initial call to next() after seeking should not move the iterator
-        // as the iterator would be positioned on the first element
-        // This behaviour allows the next() method to be used in a while-loop (eg. while iter.next())
-        if self.just_seeked {
-            self.just_seeked = false;
-            return self.valid();
-        } else {
         unsafe { ffi::rocksdb_iter_next(self.inner); }
     }
 
-        self.valid()
-    }
-
     /// Seeks to the previous key.
     ///
     /// Returns true if the iterator is valid after this operation.
-    pub fn prev(&mut self) -> bool {
+    pub fn prev(&mut self) {
-        // Initial call to prev() after seeking should not move the iterator
-        // as the iterator would be positioned on the first element
-        // This behaviour allows the prev() method to be used in a while-loop (eg. while iter.prev())
-        if self.just_seeked {
-            self.just_seeked = false;
-        } else {
         unsafe { ffi::rocksdb_iter_prev(self.inner); }
     }
 
-        self.valid()
-    }
-
     /// Returns a slice to the internal buffer storing the current key.
     ///
     /// This may be slightly more performant to use than the standard ``.key()`` method
@@ -455,6 +418,7 @@ impl DBIterator {
         let mut rv = DBIterator {
             raw: DBRawIterator::new(db, readopts),
             direction: Direction::Forward, // blown away by set_mode()
+            just_seeked: false,
         };
         rv.set_mode(mode);
         rv
@@ -468,6 +432,7 @@ impl DBIterator {
         let mut rv = DBIterator {
             raw: try!(DBRawIterator::new_cf(db, cf_handle, readopts)),
             direction: Direction::Forward, // blown away by set_mode()
+            just_seeked: false,
         };
         rv.set_mode(mode);
         Ok(rv)
@@ -489,6 +454,8 @@ impl DBIterator {
                 self.direction = dir;
             }
         };
+
+        self.just_seeked = true;
     }
 
     pub fn valid(&self) -> bool {
@@ -500,12 +467,18 @@ impl Iterator for DBIterator {
     type Item = KVBytes;
 
     fn next(&mut self) -> Option<KVBytes> {
-        let valid = match self.direction {
+        // Initial call to next() after seeking should not move the iterator
+        // or the first item will not be returned
+        if !self.just_seeked {
+            match self.direction {
                 Direction::Forward => self.raw.next(),
                 Direction::Reverse => self.raw.prev(),
-        };
+            }
+        } else {
+            self.just_seeked = false;
+        }
 
-        if valid {
+        if self.raw.valid() {
             // .key() and .value() only ever return None if valid == false, which we've just cheked
             Some((self.raw.key().unwrap().into_boxed_slice(), self.raw.value().unwrap().into_boxed_slice()))
         } else {

test/test_raw_iterator.rs

@@ -41,32 +41,20 @@ pub fn test_forwards_iteration() {
     let mut iter = db.raw_iterator();
     iter.seek_to_first();
 
-    // Shouldn't be valid yet
-    assert_eq!(iter.valid(), false);
-    assert_eq!(iter.key(), None);
-    assert_eq!(iter.value(), None);
-
-    let valid = iter.next();
-
-    assert_eq!(valid, true);
     assert_eq!(iter.valid(), true);
     assert_eq!(iter.key(), Some(b"k1".to_vec()));
     assert_eq!(iter.value(), Some(b"v1".to_vec()));
 
-    let valid = iter.next();
+    iter.next();
 
-    assert_eq!(valid, true);
     assert_eq!(iter.valid(), true);
     assert_eq!(iter.key(), Some(b"k2".to_vec()));
     assert_eq!(iter.value(), Some(b"v2".to_vec()));
 
     iter.next();  // k3
     iter.next();  // k4
+    iter.next();  // invalid!
 
-    let valid = iter.next();
-
-    // Should be invalid again
-    assert_eq!(valid, false);
     assert_eq!(iter.valid(), false);
     assert_eq!(iter.key(), None);
     assert_eq!(iter.value(), None);
@@ -84,32 +72,20 @@ pub fn test_seek_last() {
     let mut iter = db.raw_iterator();
     iter.seek_to_last();
 
-    // Shouldn't be valid yet
-    assert_eq!(iter.valid(), false);
-    assert_eq!(iter.key(), None);
-    assert_eq!(iter.value(), None);
-
-    let valid = iter.prev();
-
-    assert_eq!(valid, true);
     assert_eq!(iter.valid(), true);
     assert_eq!(iter.key(), Some(b"k4".to_vec()));
     assert_eq!(iter.value(), Some(b"v4".to_vec()));
 
-    let valid = iter.prev();
+    iter.prev();
 
-    assert_eq!(valid, true);
     assert_eq!(iter.valid(), true);
     assert_eq!(iter.key(), Some(b"k3".to_vec()));
     assert_eq!(iter.value(), Some(b"v3".to_vec()));
 
     iter.prev();  // k2
     iter.prev();  // k1
+    iter.prev();  // invalid!
 
-    let valid = iter.prev();
-
-    // Should be invalid again
-    assert_eq!(valid, false);
     assert_eq!(iter.valid(), false);
     assert_eq!(iter.key(), None);
     assert_eq!(iter.value(), None);
@@ -127,41 +103,6 @@ pub fn test_seek() {
     let mut iter = db.raw_iterator();
     iter.seek(b"k2");
 
-    // Shouldn't be valid yet
-    assert_eq!(iter.valid(), false);
-    assert_eq!(iter.key(), None);
-    assert_eq!(iter.value(), None);
-
-    let valid = iter.next();
-
-    // Should now be valid
-    assert_eq!(valid, true);
-    assert_eq!(iter.valid(), true);
-    assert_eq!(iter.key(), Some(b"k2".to_vec()));
-    assert_eq!(iter.value(), Some(b"v2".to_vec()));
-}
-
-
-#[test]
-pub fn test_seek_then_prev() {
-    let db = setup_test_db("seek_then_prev");
-    db.put(b"k1", b"v1").unwrap();
-    db.put(b"k2", b"v2").unwrap();
-    db.put(b"k3", b"v3").unwrap();
-    db.put(b"k4", b"v4").unwrap();
-
-    let mut iter = db.raw_iterator();
-    iter.seek(b"k2");
-
-    // Shouldn't be valid yet
-    assert_eq!(iter.valid(), false);
-    assert_eq!(iter.key(), None);
-    assert_eq!(iter.value(), None);
-
-    let valid = iter.prev();
-
-    // Should now be valid
-    assert_eq!(valid, true);
     assert_eq!(iter.valid(), true);
     assert_eq!(iter.key(), Some(b"k2".to_vec()));
     assert_eq!(iter.value(), Some(b"v2".to_vec()));
@@ -178,14 +119,6 @@ pub fn test_seek_to_nonexistant() {
     let mut iter = db.raw_iterator();
     iter.seek(b"k2");
 
-    // Shouldn't be valid yet
-    assert_eq!(iter.valid(), false);
-    assert_eq!(iter.key(), None);
-    assert_eq!(iter.value(), None);
-
-    let valid = iter.next();
-
-    assert_eq!(valid, true);
     assert_eq!(iter.valid(), true);
     assert_eq!(iter.key(), Some(b"k3".to_vec()));
     assert_eq!(iter.value(), Some(b"v3".to_vec()));