Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Collection.contains method #1030

Closed
DartBot opened this issue Jan 3, 2012 · 7 comments
Closed

Add Collection.contains method #1030

DartBot opened this issue Jan 3, 2012 · 7 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-obsolete Closed as the reported issue is no longer relevant

Comments

@DartBot
Copy link

DartBot commented Jan 3, 2012

This issue was originally filed by @seaneagan


There are currently several methods to determine if a collection
"contains" a value:

map.containsKey(key)
map.containsValue(value)
set.contains(item)
list.indexOf(item) != -1

I propose unifying these into a Collection.contains method, whose
default implementation would be something like:

contains (T item) {
 for(final T i in this) {
   if(item == i) {
     return true;
   }
 }
 return false;
}

I use item == i rather than i == item since the caller likely
knows and cares more about the == method of the item they pass in than
that of each item in the collection. Another option would be item === i, but I think that is a less common use case which could be done
via a separate "containsIdentical" method.

It also might make sense to shorten Map's getKeys()/getValues() to
just keys/values (getters). I assume, as in Java, that these methods
return Collections which are backed by the original Map, and thus are
O(1). Then the above examples would simplify to:

map.keys.contains(key);
map.values.contains(value);
set.contains(item);
list.contains(item);

... and even if you don't know the particular Collection sub-interface
of a collection, you can still easily test whether it contains an
item:

collection.contains(item);

List.indexOf would still be necessary for determining the exact index
at which the list contains the item, but Map.containsKey,
Map.containsValue, and Set.contains could all be removed.

FWIW, JavaScript doesn't yet have a builtin collections framework, but
the need for a "contains" method for lists is being discussed for ES6
(see https://mail.mozilla.org/pipermail/es-discuss/2011-December/019099.html),
and the most popular collections library
(http://documentcloud.github.com/underscore/) has a "contains" method
similar to what I am proposing.

@dgrove
Copy link
Contributor

dgrove commented Jan 9, 2012

Added Area-Library, Triaged labels.

@DartBot
Copy link
Author

DartBot commented Jan 20, 2012

This comment was originally written by @tomyeh


Agree that it is more consistent and avoids unnecessary typo.

In additions, Collection.remove(E) is a nice feature too.

@DartBot
Copy link
Author

DartBot commented Jan 20, 2012

This comment was originally written by @seaneagan


Filed issue #1248 separately for shortening getKeys() / getValues() to keys / values.

@DartBot
Copy link
Author

DartBot commented Jan 23, 2012

This comment was originally written by @seaneagan


Java also has Collection#contains, whose contract does indeed use item == i (see above), however, it special cases null:

item === null ? i === null : item == i

which is good, since it avoids an NPE when item === null. Also, Java does not actually require calling ==, but something equivalent to == such as comparing hashCode()'s, which is also a good idea.

Java also has Collection#containsAll, which is quite useful as well, its contract in Dart should be:

containsAll(Iterable<E> other) {
  for(final i in other) if(!contains(i)) return false;
  return true;
}

except similarly to "contains" not needing to call ==, "containsAll" should not need to call "contains", i.e. it could inline it.

@DartBot
Copy link
Author

DartBot commented Feb 10, 2012

This comment was originally written by @seaneagan


Just wanted to point out that:

http://news.dartlang.org/2012/01/proposed-changes-for-equality.html

really helps here, since the null checks are implied, and thus the contract goes back to being just:

item == i

@alan-knight
Copy link
Contributor

I'd also comment that it would be nice to allow strings to be treated like collections for some operations. Sadly, strings already have a contains method that does something different. And an indexOf that behaves differently.

@munificent
Copy link
Member

Looks like this is in Collection now.


Added AssumedStale label.

@DartBot DartBot added Type-Defect area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-obsolete Closed as the reported issue is no longer relevant labels Nov 8, 2012
dart-bot pushed a commit that referenced this issue Mar 23, 2021
2021-03-23 irina.arkhipets@gmail.com Fixes Issues #1021, #1030: tests corrected according to the roll results, missing Issue tags added.

Cq-Include-Trybots: dart/try:analyzer-nnbd-linux-release-try,dart2js-nnbd-linux-x64-chrome-try,ddc-nnbd-linux-release-chrome-try,front-end-nnbd-linux-release-x64-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-nnbd-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-x64-try,vm-kernel-nnbd-mac-release-x64-try,vm-kernel-nnbd-win-release-x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-release-x64-try
Change-Id: I2d447eb68e2bf3d0a3c8fb071ecf469a800ea6ad
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192680
Reviewed-by: William Hesse <whesse@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-obsolete Closed as the reported issue is no longer relevant
Projects
None yet
Development

No branches or pull requests

4 participants