Fix for #27: put and remove in MutableMap should return previous valu…

…e, add and remove in MutableSet should return Boolean

source/ceylon/collection/HashMap.ceylon

@@ -20,7 +20,7 @@ shared class HashMap<Key, Item>()
             while(index < store.size){
                 variable Cell<Key->Item>? bucket = store[index];
                 while(exists Cell<Key->Item> cell = bucket){
-                    addToStore(newStore, cell.car.key, cell.car.item);
+                    addToStore(newStore, cell.car.key, cell.car.item, false);
                     bucket = cell.cdr;
                 }
                 index++;
@@ -29,28 +29,29 @@ shared class HashMap<Key, Item>()
         }
     }
 
-    Boolean addToStore(Array<Cell<Key->Item>?> store, Key key, Item item){
+    Item? addToStore(Array<Cell<Key->Item>?> store, Key key, Item item, Boolean handleGrowingAndRehash){
         Integer index = storeIndex(key, store);
         variable Cell<Key->Item>? bucket = store[index];
         while(exists Cell<Key->Item> cell = bucket){
             if(cell.car.key == key){
+                Item oldValue = cell.car.item;
                 // modify an existing entry
                 cell.car = key->item;
-                return false;
+                return oldValue;
             }
             bucket = cell.cdr;
         }
         // add a new entry
         store.setItem(index, Cell<Key->Item>(key->item, store[index]));
-        return true;
-    }
-
-    doc "Adds a key/value mapping to this map, may be used to modify an existing mapping"
-    shared actual void put(Key key, Item item){
-        if(addToStore(store, key, item)){
+        if(handleGrowingAndRehash){
             _size++;
             checkRehash();
         }
+        return null;
+    }
+
+    shared actual Item? put(Key key, Item item){
+        return addToStore(store, key, item, true);
     }
 
     doc "Adds a collection of key/value mappings to this map, may be used to change existing mappings"
@@ -61,7 +62,7 @@ shared class HashMap<Key, Item>()
     }
 
     doc "Removes a key/value mapping if it exists"
-    shared actual void remove(Key key){
+    shared actual Item? remove(Key key){
         Integer index = storeIndex(key, store);
         variable Cell<Key->Item>? bucket = store[index];
         variable Cell<Key->Item>? prev = null;
@@ -74,11 +75,12 @@ shared class HashMap<Key, Item>()
                     store.setItem(index, cell.cdr);
                 }
                 _size--;
-                return;
+                return cell.car.item;
             }
             prev = cell;
             bucket = cell.cdr;
         }
+        return null;
     }
 
     doc "Removes every key/value mapping"

source/ceylon/collection/HashSet.ceylon

@@ -44,24 +44,25 @@ shared class HashSet<Element>()
         return true;
     }
 
-    doc "Adds an element to this set, if not already present"
-    shared actual void add(Element element){
+    shared actual Boolean add(Element element){
         if(addToStore(store, element)){
             _size++;
             checkRehash();
+            return true;
         }
+        return false;
     }
 
-    doc "Adds every element from the given collection to this set, unless already present"
-    shared actual void addAll(Element* elements){
+    shared actual Boolean addAll(Element* elements){
+        variable Boolean ret = false;
         for(Element elem in elements){
-            add(elem);
+            ret ||= add(elem);
         }
         checkRehash();
+        return ret;
     }
 
-    doc "Removes an element from this set, if present"
-    shared actual void remove(Element element){
+    shared actual Boolean remove(Element element){
         Integer index = storeIndex(element, store);
         variable Cell<Element>? bucket = store[index];
         variable Cell<Element>? prev = null;
@@ -74,11 +75,12 @@ shared class HashSet<Element>()
                     store.setItem(index, cell.cdr);
                 }
                 _size--;
-                return;
+                return true;
             }
             prev = cell;
             bucket = cell.cdr;
         }
+        return false;
     }
 
     doc "Removes every element"

source/ceylon/collection/MutableMap.ceylon

@@ -5,15 +5,19 @@ shared interface MutableMap<Key, Item>
         given Key satisfies Object 
         given Item satisfies Object {
 
-    doc "Adds a key/value mapping to this map, may be used to modify an existing mapping"
-    shared formal void put(Key key, Item item);
+    doc "Adds a key/value mapping to this map, may be used to modify an existing mapping.
+         
+         Returns the previous value pointed to by `key`, or null."
+    shared formal Item? put(Key key, Item item);
 
-    doc "Adds the key/value mappings to this map, may be used to change existing mappings"
+    doc "Adds the key/value mappings to this map, may be used to change existing mappings."
     shared formal void putAll(<Key->Item>* entries);
 
-    doc "Removes a key/value mapping if it exists"
-    shared formal void remove(Key key);
+    doc "Removes a key/value mapping if it exists.
+         
+         Returns the previous value pointed to by `key`, or null."
+    shared formal Item? remove(Key key);
 
-    doc "Removes every key/value mapping"
+    doc "Removes every key/value mapping."
     shared formal void clear();
 }

source/ceylon/collection/MutableSet.ceylon

@@ -4,15 +4,21 @@ shared interface MutableSet<Element>
     satisfies Set<Element>
         given Element satisfies Object {
 
-    doc "Adds an element to this set, if not already present"
-    shared formal void add(Element element);
+    doc "Adds an element to this set, if not already present. 
+         
+         Returns true if the element was added, false if it was already part of the Set."
+    shared formal Boolean add(Element element);
 
-    doc "Adds the elements to this set, unless already present"
-    shared formal void addAll(Element* elements);
+    doc "Adds the elements to this set, unless already present. 
+         
+         Returns true if any element was added, false if they were all already part of the Set."
+    shared formal Boolean addAll(Element* elements);
 
-    doc "Removes an element from this set, if present"
-    shared formal void remove(Element element);
+    doc "Removes an element from this set, if present.
+         
+         Returns true if the element was removed, false if it was not part of the Set."
+    shared formal Boolean remove(Element element);
 
-    doc "Removes every element"
+    doc "Removes every element."
     shared formal void clear();
 }

source/ceylon/json/Object.ceylon

@@ -42,16 +42,16 @@ shared class Object({Entry<String, String|Boolean|Integer|Float|Object|Array|Nul
         return contents.iterator();
     }
 
-    shared actual void put(String key, String|Boolean|Integer|Float|Object|Array|NullInstance item) {
-        contents.put(key, item);
+    shared actual Null|String|Boolean|Integer|Float|Object|Array|NullInstance put(String key, String|Boolean|Integer|Float|Object|Array|NullInstance item) {
+        return contents.put(key, item);
     }
 
     shared actual void putAll(Entry<String,String|Boolean|Integer|Float|Object|Array|NullInstance>* entries) {
         contents.putAll(*entries);
     }
 
-    shared actual void remove(String key) {
-        contents.remove(key);
+    shared actual Null|String|Boolean|Integer|Float|Object|Array|NullInstance remove(String key) {
+        return contents.remove(key);
     }
 
     shared actual Integer size {

test-source/test/ceylon/collection/map.ceylon

@@ -7,12 +7,12 @@ void testMap(){
     assertEquals(0, map.size);
     assertTrue(!map.defines("fu"), "a");
 
-    map.put("fu", "bar");
+    assertEquals(null, map.put("fu", "bar"));
     assertEquals("{fu->bar}", map.string);
     assertEquals(1, map.size);
     assertTrue(map.defines("fu"), "b");
 
-    map.put("fu", "gee");
+    assertEquals("bar", map.put("fu", "gee"));
     assertEquals("{fu->gee}", map.string);
     assertEquals(1, map.size);
     assertTrue(map.defines("fu"), "c");
@@ -39,15 +39,17 @@ void testMapRemove(){
     map.put("c", "d");
     assertEquals(2, map.size);
 
-    map.remove("a");
+    assertEquals("b", map.remove("a"));
     assertEquals(1, map.size);
     assertEquals("d", map["c"]);
     assertEquals(null, map["a"]);
 
-    map.remove("c");
+    assertEquals("d", map.remove("c"));
     assertEquals(0, map.size);
     assertEquals(null, map["c"]);
     assertEquals(null, map["a"]);
+
+    assertEquals(null, map.remove("c"));
 }
 
 void testMap2(){

test-source/test/ceylon/collection/set.ceylon

@@ -6,12 +6,12 @@ void testSet(){
     assertEquals("()", set.string);
     assertEquals(0, set.size);
 
-    set.add("fu");
+    assertEquals(true, set.add("fu"));
     assertEquals("(fu)", set.string);
     assertTrue(set.contains("fu"));
     assertEquals(1, set.size);
 
-    set.add("fu");
+    assertEquals(false, set.add("fu"));
     assertEquals("(fu)", set.string);
     assertTrue(set.contains("fu"));
     assertEquals(1, set.size);
@@ -35,15 +35,17 @@ void testSetRemove(){
     set.add("b");
     assertEquals(2, set.size);
 
-    set.remove("a");
+    assertEquals(true, set.remove("a"));
     assertEquals(1, set.size);
     assertFalse(set.contains("a"));
     assertTrue(set.contains("b"));
 
-    set.remove("b");
+    assertEquals(true, set.remove("b"));
     assertEquals(0, set.size);
     assertFalse(set.contains("a"));
     assertFalse(set.contains("b"));
+
+    assertEquals(false, set.remove("b"));
 }
 
 void testSet2(){