Make the documentation of ListIterator.add and BidirList.insert more specific.

Also fix the implementation of them in other methods to make it consistent.

6 files changed, 203 insertions, 125 deletions

diff --git a/gee/arraylist.vala b/gee/arraylist.vala
index 2943162..0cbad76 100644
--- a/gee/arraylist.vala
+++ b/gee/arraylist.vala
@@ -44,7 +44,7 @@ public class Gee.ArrayList<G> : AbstractBidirList<G> {
 	public override int size {
 		get { return _size; }
 	}
-	
+
 	/**
 	 * {@inheritDoc}
 	 */
@@ -311,7 +311,9 @@ public class Gee.ArrayList<G> : AbstractBidirList<G> {
 	}
 
 	private void set_capacity (int value) {
+#if !DISABLE_INTERNAL_ASSERTS
 		assert (value >= _size);
+#endif
 
 		_items.resize (value);
 	}
@@ -356,17 +358,24 @@ public class Gee.ArrayList<G> : AbstractBidirList<G> {
 
 		public new G get () {
 			assert (_stamp == _list._stamp);
+			assert (! _removed);
 			assert (_index >= 0);
+#if !DISABLE_INTERNAL_ASSERTS
 			assert (_index < _list._size);
-			assert (! _removed);
+#else
+			assume (_index < _list._size);
+#endif
 			return _list._items[_index];
 		}
 
 		public void remove () {
 			assert (_stamp == _list._stamp);
-			assert (_index >= 0);
+			assert (! _removed && _index >= 0);
+#if !DISABLE_INTERNAL_ASSERTS
 			assert (_index < _list._size);
-			assert (! _removed);
+#else
+			assume (_index < _list._size);
+#endif
 			_list.remove_at (_index);
 			_index--;
 			_removed = true;
@@ -375,6 +384,10 @@ public class Gee.ArrayList<G> : AbstractBidirList<G> {
 
 		public bool previous () {
 			assert (_stamp == _list._stamp);
+			if (_removed && _index >= 0) {
+				_removed = false;
+				return true;
+			}
 			if (_index > 0) {
 				_index--;
 				return true;
@@ -384,7 +397,7 @@ public class Gee.ArrayList<G> : AbstractBidirList<G> {
 
 		public bool has_previous () {
 			assert (_stamp == _list._stamp);
-			return (_index - 1 >= 0);
+			return (_index > 0 || (_removed && _index >= 0));
 		}
 
 		public bool last () {
@@ -398,27 +411,39 @@ public class Gee.ArrayList<G> : AbstractBidirList<G> {
 
 		public new void set (G item) {
 			assert (_stamp == _list._stamp);
+			assert (! _removed);
 			assert (_index >= 0);
+#if !DISABLE_INTERNAL_ASSERTS
 			assert (_index < _list._size);
+#else
+			assume (_index < _list._size);
+#endif
 			_list._items[_index] = item;
 			_stamp = ++_list._stamp;
 		}
 
 		public void insert (G item) {
 			assert (_stamp == _list._stamp);
-			assert (_index >= 0);
 			assert (_index < _list._size);
-			_list.insert (_index, item);
+			if (_index == -1) {
+				_list.insert (0, item);
+				_removed = true;
+			}
+			if (_removed) {
+				_list.insert (_index + 1, item);
+			} else {
+				_list.insert (_index, item);
+			}
 			_index++;
 			_stamp = _list._stamp;
 		}
 
 		public void add (G item) {
 			assert (_stamp == _list._stamp);
-			assert (_index >= 0);
 			assert (_index < _list._size);
 			_list.insert (_index + 1, item);
 			_index++;
+			_removed = false;
 			_stamp = _list._stamp;
 		}
 
diff --git a/gee/bidirlistiterator.vala b/gee/bidirlistiterator.vala
index edb858b..9897e79 100644
--- a/gee/bidirlistiterator.vala
+++ b/gee/bidirlistiterator.vala
@@ -23,7 +23,11 @@
 public interface Gee.BidirListIterator<G> : Gee.BidirIterator<G>, Gee.ListIterator<G> {
 	/**
 	 * Inserts the specified item before the current item in the iteration. The
-	 * cursor is let to point to the current item.
+	 * iterator points to the same element as before.
+	 *
+	 * Please note that if iterator points in-between elements the element
+	 * is added between neighbouring elements and the iterator point between
+	 * added element and the next one.
 	 */
 	public abstract void insert (G item);
 }
diff --git a/gee/concurrentlist.vala b/gee/concurrentlist.vala
index 106f4db..7863fa8 100644
--- a/gee/concurrentlist.vala
+++ b/gee/concurrentlist.vala
@@ -369,6 +369,7 @@ public class Gee.ConcurrentList<G> : AbstractList<G> {
 			new_node.insert (_prev, _curr);
 			_curr = (owned)new_node;
 			_index++;
+			_removed = false;
 		}
 
 		public new bool foreach (ForallFunc<G> f) {
diff --git a/gee/iterator.vala b/gee/iterator.vala
index b22fa10..99ec295 100644
--- a/gee/iterator.vala
+++ b/gee/iterator.vala
@@ -32,8 +32,13 @@
  * item has been removed, until the next call to {@link next}.
  *
  * Please note that when the iterator is out of track, neither {@link get} nor
- * {@link remove} are defined and both will fail. After the next call to
+ * {@link remove} are defined and both might fail. After the next call to
  * {@link next}, they will be defined again.
+ *
+ * Please also note that, unless specified otherwise, iterators before iteration
+ * started should behave as if after deletion of the first element. Whenever
+ * documentation states about the iterator 'out of track', 'invalid' or
+ * 'in-between elements' this refers to the same concept.
  */
 public interface Gee.Iterator<G> : Object, Traversable<G> {
 	/**
@@ -63,13 +68,13 @@ public interface Gee.Iterator<G> : Object, Traversable<G> {
 	 * the next move of the cursor (calling {@link next}).
 	 */
 	public abstract void remove ();
-	
+
 	/**
 	 * Determines wheather the call to {@link get} is legal. It is false at the
 	 * beginning and after {@link remove} call and true otherwise.
 	 */
 	public abstract bool valid { get; }
-	
+
 	/**
 	 * Determines wheather the call to {@link remove} is legal assuming the
 	 * iterator is valid. The value must not change in runtime hence the user
diff --git a/gee/linkedlist.vala b/gee/linkedlist.vala
index 061426d..fa3fdf7 100644
--- a/gee/linkedlist.vala
+++ b/gee/linkedlist.vala
@@ -70,7 +70,7 @@ public class Gee.LinkedList<G> : AbstractBidirList<G>, Queue<G>, Deque<G> {
 	private LinkedList.with_closures (owned Functions.EqualDataFuncClosure<G> equal_func) {
 		_equal_func = equal_func;
 	}
-	
+
 	~LinkedList () {
 		this.clear ();
 	}
@@ -446,16 +446,15 @@ public class Gee.LinkedList<G> : AbstractBidirList<G>, Queue<G>, Deque<G> {
 	}
 
 	private class Iterator<G> : Object, Traversable<G>, Gee.Iterator<G>, BidirIterator<G>, ListIterator<G>, BidirListIterator<G> {
-		private bool started = false;
-		private bool removed = false;
-		private unowned Node<G>? position;
+		private bool _removed = false;
+		private unowned Node<G>? _position;
 		private int _stamp;
 		private LinkedList<G> _list;
 		private int _index;
 
 		public Iterator (LinkedList<G> list) {
 			this._list = list;
-			this.position = null;
+			this._position = null;
 			this._index = -1;
 			this._stamp = list._stamp;
 		}
@@ -463,200 +462,241 @@ public class Gee.LinkedList<G> : AbstractBidirList<G>, Queue<G>, Deque<G> {
 		public bool next () {
 			assert (this._stamp == this._list._stamp);
 
-			if (this.removed) {
-				if (this.position != null) {
-					this.removed = false;
-					return true;
-				} else {
-					return false;
-				}
-			} else if (!this.started) {
-				if (this._list._head != null) {
-					this.started = true;
-					this.position = this._list._head;
-					this._index++;
+			if (GLib.unlikely (_position == null)) {
+#if !DISABLE_INTERNAL_ASSERTS
+				assert (!_removed);
+#else
+				assume (!_removed);
+#endif
+				if (_list._head != null) {
+					_position = _list._head;
+					_index = 0;
 					return true;
 				} else {
 					return false;
 				}
-			} else if (this.position != null) {
-				if (this.position.next != null) {
-					this.position = this.position.next;
-					this._index++;
+			} else {
+				if (_position.next != null) {
+					_position = _position.next;
+					_index++;
+					_removed = false;
 					return true;
 				} else {
 					return false;
 				}
 			}
-			return false;
 		}
 
 		public bool has_next () {
-			assert (this._stamp == this._list._stamp);
+			assert (_stamp == _list._stamp);
 
-			if (this.removed) {
-				return this.position != null;
-			} else if (!this.started) {
-				return this._list._head != null;
-			} else if (this.position != null) {
-				return this.position.next != null;
+			if (GLib.unlikely (_position == null)) {
+				return _list._head != null;
+			} else {
+				return _position.next != null;
 			}
-			return false;
 		}
 
 		public bool first () {
-			assert (this._stamp == this._list._stamp);
-			if (this._list.size == 0) {
+			assert (_stamp == _list._stamp);
+
+			if (_list.size == 0) {
 				return false;
 			}
-			this.position = this._list._head;
-			this.started = true;
-			this._index = 0;
-			this.removed = false;
-			return this.position != null;
+			_position = _list._head;
+			_index = 0;
+			_removed = false;
+#if !DISABLE_INTERNAL_ASSERTS
+			assert (_position != null);
+#endif
+			return true;
 		}
 
 		public new G get () {
-			assert (this._stamp == this._list._stamp);
-			assert (this.position != null);
+			assert (_stamp == _list._stamp);
+			assert (_position != null && !_removed);
 
-			return this.position.data;
+			return _position.data;
 		}
 
 		public void remove () {
-			assert (this._stamp == this._list._stamp);
-			assert (this.position != null);
+			assert (_stamp == _list._stamp);
+			assert (_position != null && !_removed);
 
-			unowned Node<G>? new_position = this.position.next;
-			if (new_position == null) {
-				started = false;
+			unowned Node<G>? new_position = _position.prev;
+			_list._remove_node (_position);
+			_position = new_position;
+			if (_position != null) {
+				_removed = true;
 			}
-			_list._remove_node (this.position);
-			this.position = new_position;
-			this.removed = true;
-			this._stamp = this._list._stamp;
+			_index--;
+			_stamp = _list._stamp;
 		}
 
 		public bool previous () {
-			assert (this._stamp == this._list._stamp);
+			assert (_stamp == _list._stamp);
 
-			if (!this.started) {
-				this.position = null;
+			if (GLib.likely (_position != null)) {
+				if (GLib.unlikely (_removed)) {
+					_removed = false;
+					return true;
+				} else if (GLib.likely(_position.prev != null)) {
+					_position = _position.prev;
+					_index--;
+					return true;
+				} else {
+					return false;
+				}
+			} else {
 				return false;
-			} else if (this.position != null && this.position.prev != null) {
-				this.position = this.position.prev;
-				this._index--;
-				return true;
 			}
-			return false;
 		}
 
 		public bool has_previous () {
-			assert (this._stamp == this._list._stamp);
+			assert (_stamp == _list._stamp);
 
-			if (!this.started) {
+			if (GLib.likely (_position != null)) {
+				if (GLib.unlikely (_removed)) {
+					return true;
+				} else {
+					return _position.prev != null;
+				}
+			} else {
 				return false;
-			} else if (this.position != null) {
-				return this.position.prev != null;
 			}
-			return false;
 		}
 
 		public bool last () {
-			assert (this._stamp == this._list._stamp);
+			assert (_stamp == _list._stamp);
 
-			if (this._list.size == 0) {
+			if (_list.size == 0) {
 				return false;
 			}
-			this.position = this._list._tail;
-			this.started = true;
-			this._index = this._list._size - 1;
-			return this.position != null;
+			_position = _list._tail;
+			_index = _list._size - 1;
+#if !DISABLE_INTERNAL_ASSERTS
+			assert (_position != null);
+#endif
+			return true;
 		}
 
 		public new void set (G item) {
-			assert (this._stamp == this._list._stamp);
-			assert (this.position != null);
+			assert (_stamp == _list._stamp);
+			assert (_position != null && !_removed);
 
-			this.position.data = item;
+			_position.data = item;
 		}
 
 		public void insert (G item) {
-			assert (this._stamp == this._list._stamp);
-			assert (this.position != null);
+			assert (_stamp == _list._stamp);
 
 			Node<G> n = new Node<G> (item);
-			if (this.position.prev != null) {
-				Node<G> position = (owned) this.position.prev.next;
-				n.prev = position.prev;
-				position.prev = n;
-				n.next = (owned) position;
-				weak Node<G> _n = n;
-				_n.prev.next = (owned) n;
+			unowned Node<G> n_ref = n;
+			if (_position == null) {
+				Node<G>? position = (owned)_list._head;
+				if (position != null) {
+					position.prev = n;
+					n.next = (owned)position;
+				} else {
+#if !DISABLE_INTERNAL_ASSERTS
+					assert (_list._tail == null);
+#endif
+					_list._tail = n;
+				}
+				if (_position == null) {
+					_position = n_ref;
+				}
+				_list._head = (owned)n;
 			} else {
-				Node<G> position = (owned) this._list._head;
-				position.prev = n;
-				n.next = (owned) position;
-				this._list._head = (owned) n;
+				if (_removed) {
+					if (_position.next != null) {
+						n.next = (owned)_position.next;
+						n.next.prev = n;
+					} else {
+						_list._tail = n;
+					}
+					n.prev = _position;
+					_position.next = (owned)n;
+					_position = n_ref;
+				} else {
+					n.prev = _position.prev;
+					_position.prev = n;
+					if (n.prev != null) {
+						n.next = (owned)n.prev.next;
+						n.prev.next = (owned)n;
+					} else {
+						n.next = (owned)_list._head;
+						_list._head = (owned)n;
+					}
+				}
 			}
-			this._list._size++;
-			this._index++;
+			_list._size++;
+			_index++;
 			_stamp = _list._stamp;
 		}
 
 		public void add (G item) {
-			assert (this._stamp == this._list._stamp);
-			assert (this.position != null);
+			assert (_stamp == _list._stamp);
 
 			Node<G> n = new Node<G> (item);
-			if (this.position.next != null) {
-				this.position.next.prev = n;
-				n.next = (owned) this.position.next;
+			unowned Node<G> n_ref = n;
+			if (_position == null) {
+				Node<G> position = (owned)_list._head;
+				position.prev = n;
+				n.next = (owned)position;
+				_list._head = (owned) n;
 			} else {
-				this._list._tail = n;
+				if (_position.next != null) {
+					_position.next.prev = n;
+					n.next = (owned)_position.next;
+				} else {
+					_list._tail = n;
+				}
+				_position.next = (owned)n;
+				_position.next.prev = _position;
 			}
-			this.position.next = (owned) n;
-			this.position.next.prev = this.position;
-			this.position = this.position.next;
-			this._list._size++;
-			this._index++;
+			_position = n_ref;
+			_removed = false;
+			_list._size++;
+			_index++;
 			_stamp = _list._stamp;
 		}
 
 		public int index () {
-			assert (this._stamp == this._list._stamp);
-			assert (this.position != null);
+			assert (_stamp == _list._stamp);
+			assert (_position != null && !_removed);
 
-			return this._index;
+			return _index;
 		}
-		
+
 		public bool read_only {
 			get {
 				return false;
 			}
 		}
-		
+
 		public bool valid {
 			get {
-				return !this.removed && this.position != null;
+				return !_removed && _position != null;
 			}
 		}
 
 		public bool foreach (ForallFunc<G> f) {
 			assert (_stamp == _list._stamp);
-			if (!started) {
-				position = _list._head;
-				if (position != null)
-					started = true;
+			if (_position == null) {
+				_position = _list._head;
+			}
+			if (_removed) {
+				_position = _position.next;
+				_removed = false;
 			}
-			removed = false;
-			while (position != null) {
-				if (!f (position.data)) {
+			while (_position != null) {
+				if (!f (_position.data)) {
 					return false;
 				}
-				position = position.next;
+				_position = _position.next;
 			}
-			position = _list._tail;
+			_position = _list._tail;
 			return true;
 		}
 	}
diff --git a/gee/listiterator.vala b/gee/listiterator.vala
index 2dfd0e9..e59486d 100644
--- a/gee/listiterator.vala
+++ b/gee/listiterator.vala
@@ -31,7 +31,10 @@ public interface Gee.ListIterator<G> : Gee.Iterator<G> {
 
 	/**
 	 * Adds the specified item after the current item in the iteration. The
-	 * cursor is moved to point to the new added item.
+	 * iterator is moved to the point of the new added item.
+	 *
+	 * Please note that if iterator points in-between elements the element
+	 * is added after the current element and iterator point on it.
 	 */
 	public abstract void add (G item);