Set.add returns true if item has been added, otherwise false

BUG= https://code.google.com/p/dart/issues/detail?id=3546
R=floitsch@google.com

Review URL: https://codereview.chromium.org//26280002

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@28863 260f80e4-7a28-3924-810f-c04153c831b5

pkg/unmodifiable_collection/lib/unmodifiable_collection.dart

@@ -185,7 +185,7 @@ class UnmodifiableSetView<E> extends _IterableView<E>
    * Throws an [UnsupportedError];
    * operations that change the set are disallowed.
    */
-  void add(E value) => _throw();
+  bool add(E value) => _throw();
 
   /**
    * Throws an [UnsupportedError];

runtime/lib/collection_patch.dart

@@ -583,15 +583,16 @@ class _HashSet<E> extends _HashSetBase<E> implements HashSet<E> {
 
   // Set.
 
-  void add(E element) {
+  bool add(E element) {
     int hashCode = _hashCode(element);
     int index = hashCode & (_buckets.length - 1);
     HashSetEntry entry = _buckets[index];
     while (entry != null) {
-      if (_equals(entry.key, element)) return;
+      if (_equals(entry.key, element)) return false;
       entry = entry.next;
     }
     _addEntry(element, hashCode, index);
+    return true;
   }
 
   void addAll(Iterable<E> objects) {

sdk/lib/_internal/lib/collection_patch.dart

@@ -1002,15 +1002,15 @@ class _HashSet<E> extends _HashSetBase<E> implements HashSet<E> {
   }
 
   // Collection.
-  void add(E element) {
+  bool add(E element) {
     if (_isStringElement(element)) {
       var strings = _strings;
       if (strings == null) _strings = strings = _newHashTable();
-      _addHashTableEntry(strings, element);
+      return _addHashTableEntry(strings, element);
     } else if (_isNumericElement(element)) {
       var nums = _nums;
       if (nums == null) _nums = nums = _newHashTable();
-      _addHashTableEntry(nums, element);
+      return _addHashTableEntry(nums, element);
     } else {
       var rest = _rest;
       if (rest == null) _rest = rest = _newHashTable();
@@ -1020,11 +1020,12 @@ class _HashSet<E> extends _HashSetBase<E> implements HashSet<E> {
         _setTableEntry(rest, hash, JS('var', '[#]', element));
       } else {
         int index = _findBucketIndex(bucket, element);
-        if (index >= 0) return;
+        if (index >= 0) return false;
         JS('void', '#.push(#)', bucket, element);
       }
       _length++;
       _elements = null;
+      return true;
     }
   }
 
@@ -1131,11 +1132,12 @@ class _HashSet<E> extends _HashSetBase<E> implements HashSet<E> {
     return _elements = result;
   }
 
-  void _addHashTableEntry(var table, E element) {
-    if (_hasTableEntry(table, element)) return;
+  bool _addHashTableEntry(var table, E element) {
+    if (_hasTableEntry(table, element)) return false;
     _setTableEntry(table, element, 0);
     _length++;
     _elements = null;
+    return true;
   }
 
   bool _removeHashTableEntry(var table, Object element) {
@@ -1456,15 +1458,15 @@ class _LinkedHashSet<E> extends _HashSetBase<E> implements LinkedHashSet<E> {
   }
 
   // Collection.
-  void add(E element) {
+  bool add(E element) {
     if (_isStringElement(element)) {
       var strings = _strings;
       if (strings == null) _strings = strings = _newHashTable();
-      _addHashTableEntry(strings, element);
+      return _addHashTableEntry(strings, element);
     } else if (_isNumericElement(element)) {
       var nums = _nums;
       if (nums == null) _nums = nums = _newHashTable();
-      _addHashTableEntry(nums, element);
+      return _addHashTableEntry(nums, element);
     } else {
       var rest = _rest;
       if (rest == null) _rest = rest = _newHashTable();
@@ -1475,10 +1477,11 @@ class _LinkedHashSet<E> extends _HashSetBase<E> implements LinkedHashSet<E> {
         _setTableEntry(rest, hash, JS('var', '[#]', cell));
       } else {
         int index = _findBucketIndex(bucket, element);
-        if (index >= 0) return;
+        if (index >= 0) return false;
         LinkedHashSetCell cell = _newLinkedCell(element);
         JS('void', '#.push(#)', bucket, cell);
       }
+      return true;
     }
   }
 
@@ -1548,10 +1551,11 @@ class _LinkedHashSet<E> extends _HashSetBase<E> implements LinkedHashSet<E> {
     }
   }
 
-  void _addHashTableEntry(var table, E element) {
+  bool _addHashTableEntry(var table, E element) {
     LinkedHashSetCell cell = _getTableEntry(table, element);
-    if (cell != null) return;
+    if (cell != null) return false;
     _setTableEntry(table, element, _newLinkedCell(element));
+    return true;
   }
 
   bool _removeHashTableEntry(var table, Object element) {

sdk/lib/core/set.dart

@@ -59,11 +59,11 @@ abstract class Set<E> extends IterableBase<E> implements EfficientLength {
   bool contains(Object value);
 
   /**
-   * Adds [value] into the set.
+   * Adds [value] into the set. Returns `true` if [value] was added to the set.
    *
-   * The method has no effect if [value] is already in the set.
+   * If [value] already exists, the set is not changed and `false` is returned.
    */
-  void add(E value);
+  bool add(E value);
 
   /**
    * Adds all of [elements] to this Set.

sdk/lib/html/dart2js/html_dart2js.dart

@@ -29045,8 +29045,10 @@ abstract class CssClassSet implements Set<String> {
    *
    * This is the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
+   *
+   * Returns `true` if [value] was added to the set, otherwise `false`.
    */
-  void add(String value);
+  bool add(String value);
 
   /**
    * Remove the class [value] from element, and return true on successful
@@ -29121,7 +29123,7 @@ class _MultiElementCssClassSet extends CssClassSetImpl {
    *   After f returns, the modified set is written to the
    *       className property of this element.
    */
-  void modify( f(Set<String> s)) {
+  modify( f(Set<String> s)) {
     _elementCssClassSetIterable.forEach((e) => e.modify(f));
   }
 

sdk/lib/html/dartium/html_dartium.dart

@@ -31127,8 +31127,10 @@ abstract class CssClassSet implements Set<String> {
    *
    * This is the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
+   *
+   * Returns `true` if [value] was added to the set, otherwise `false`.
    */
-  void add(String value);
+  bool add(String value);
 
   /**
    * Remove the class [value] from element, and return true on successful
@@ -31203,7 +31205,7 @@ class _MultiElementCssClassSet extends CssClassSetImpl {
    *   After f returns, the modified set is written to the
    *       className property of this element.
    */
-  void modify( f(Set<String> s)) {
+  modify( f(Set<String> s)) {
     _elementCssClassSetIterable.forEach((e) => e.modify(f));
   }
 

sdk/lib/html/html_common/css_class_set.dart

@@ -92,10 +92,10 @@ abstract class CssClassSetImpl implements CssClassSet {
    * This is the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
    */
-  void add(String value) {
+  bool add(String value) {
     // TODO - figure out if we need to do any validation here
     // or if the browser natively does enough.
-    modify((s) => s.add(value));
+    return modify((s) => s.add(value));
   }
 
   /**
@@ -206,10 +206,11 @@ abstract class CssClassSetImpl implements CssClassSet {
    *   After f returns, the modified set is written to the
    *       className property of this element.
    */
-  void modify( f(Set<String> s)) {
+  modify( f(Set<String> s)) {
     Set<String> s = readClasses();
-    f(s);
+    var ret = f(s);
     writeClasses(s);
+    return ret;
   }
 
   /**

tests/corelib/set_test.dart

@@ -17,15 +17,19 @@ void testInts(Set create()) {
   Set set = create();
 
   testLength(0, set);
-  set.add(1);
+  Expect.isTrue(set.add(1));
   testLength(1, set);
   Expect.isTrue(set.contains(1));
 
-  set.add(1);
+  Expect.isFalse(set.add(1));
   testLength(1, set);
   Expect.isTrue(set.contains(1));
 
-  set.remove(1);
+  Expect.isTrue(set.remove(1));
+  testLength(0, set);
+  Expect.isFalse(set.contains(1));
+
+  Expect.isFalse(set.remove(1));
   testLength(0, set);
   Expect.isFalse(set.contains(1));
 
@@ -184,7 +188,7 @@ void testInts(Set create()) {
   // Test Set.clear.
   set.clear();
   testLength(0, set);
-  set.add(11);
+  Expect.isTrue(set.add(11));
   testLength(1, set);
 }
 
@@ -205,8 +209,8 @@ void testStrings(Set create()) {
   testLength(8, set);
   set.removeAll(strings.where((x) => x.length == 3));
   testLength(4, set);
-  set.add("bar");
-  set.add("qux");
+  Expect.isTrue(set.add("bar"));
+  Expect.isTrue(set.add("qux"));
   testLength(6, set);
   set.addAll(strings);
   testLength(8, set);

tests/html/element_classes_test.dart

@@ -107,10 +107,12 @@ main() {
 
   test('add', () {
     final classes = makeClassSet();
-    classes.add('qux');
+    var added = classes.add('qux');
+    expect(added, isTrue);
     expect(classes, orderedEquals(['foo', 'bar', 'baz', 'qux']));
 
-    classes.add('qux');
+    added = classes.add('qux');
+    expect(added, isFalse);
     final list = new List.from(classes);
     list.sort((a, b) => a.compareTo(b));
     expect(list, orderedEquals(['bar', 'baz', 'foo', 'qux']),
@@ -235,7 +237,9 @@ main() {
 
   test('listAdd', () {
     var elements =  listElementSetup();
-    elements.classes.add('lassie');
+    var added = elements.classes.add('lassie');
+    expect(added, isNull);
+
     expect(elements.classes,
         unorderedEquals(['lassie', 'qux', 'quux', 'meta', 'classy', 'lassy']));
     for (Element e in elements) {

tools/dom/src/CssClassSet.dart

@@ -35,8 +35,13 @@ abstract class CssClassSet implements Set<String> {
    *
    * This is the Dart equivalent of jQuery's
    * [addClass](http://api.jquery.com/addClass/).
+   *
+   * If this corresponds to one element. Returns `true` if [value] was added to
+   * the set, otherwise `false`.
+   *
+   * If this corresponds to many elements, `null` is always returned.
    */
-  void add(String value);
+  bool add(String value);
 
   /**
    * Remove the class [value] from element, and return true on successful
@@ -111,7 +116,7 @@ class _MultiElementCssClassSet extends CssClassSetImpl {
    *   After f returns, the modified set is written to the
    *       className property of this element.
    */
-  void modify( f(Set<String> s)) {
+  modify( f(Set<String> s)) {
     _elementCssClassSetIterable.forEach((e) => e.modify(f));
   }