search

allalizaki / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
1 stars 0 forks source link

Itera*s.getFirst(Iterator<T>, T) - and add the ", T" overloads for get/getLast/getOnlyElement #217

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
There is a nice Iterables.getLast(..) method, however an
Iterables.getFirst(..) method would be also nice (that means also on
Iterators.class of course).

Currently you have to call getElement(.., 0), which reads not very nice.

When adding this method, having an variation getFirstOrDefault(..) as
somewhere discussed here (Issue 150) for getLast(..) seems like a natural
addition.

See also: http://code.google.com/p/google-collections/issues/detail?id=150

Original issue reported on code.google.com by sebastian.jancke@googlemail.com on 11 Aug 2009 at 8:53

GoogleCodeExporter commented 9 years ago 
getFirst(Iterator<T>, T defaultValue) is the only version of this that's worth
having, as without a default, you're better off just calling
theIterable.iterator().next().

Original comment by kevin...@gmail.com on 11 Aug 2009 at 1:26

GoogleCodeExporter commented 9 years ago 
>as without a default, you're better off just calling
>theIterable.iterator().next().

Calling iterable.iterator().next() is simply not readable - sorry. It is 
failing in
the same way as most of the JDK collection libraries fail (too low level, 
missing
readability, ...).

Original comment by sebastian.jancke@googlemail.com on 12 Aug 2009 at 7:20

GoogleCodeExporter commented 9 years ago 
Another issue about this: consistency. There is getLast(iterable/iterator). 
Either
you break the API to getLast(.., default) or you have to be consistent with 
getFirst.

Original comment by sebastian.jancke@googlemail.com on 12 Aug 2009 at 7:24

GoogleCodeExporter commented 9 years ago 
I couldn't disagree more on either count.

iterable.iterator().next() is perfectly clear and readable and _unambiguous_.  
I know 
exactly what it does, whereas with Iterators.getFirst(), I have to run off and 
look 
up how that library designer decided to do it.

Also, your notion of consistency is deeply misguided.  We use consistency in 
how we 
present important functionality, but we never use it to justify adding 
worthless 
functionality, and you shouldn't in your own libraries either!

Original comment by kevin...@gmail.com on 12 Aug 2009 at 5:01

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 12 Aug 2009 at 5:55

GoogleCodeExporter commented 9 years ago 
To me and collegues, "iterable.iterator().next()" is not readable. It says: 
"give me
an iterator and then the next element, that is the first". That does not comply 
with
the sense of readability stated by RC Martin and others. It is not ambigous, 
you are
right. But "getFirst" states clearly what it does - retrieving the first 
element. 

I agree on not adding worthless functionality. Maybe highest readability is not 
a
design goal of this library (I do not judge such a decision...)?

Still, having getFirst ist consistent with having getLast. It was the first 
thing I
noticed and missed, when using getLast on a collection. Maybe it's because I'm 
used
to it from .NET/LINQ code. There you have a whole bunch of clearly readabily
high-level functions on collections - getFirst is one of them and lot's of 
others are
already part of this library.

In the end - It was just a suggestion and expression of my experience with this 
very
nice library. I won't comment more on this, maybe you implement it or you 
decide not
to - both is ok (as I can still add getFirst through AJ).

Original comment by sebastian.jancke@googlemail.com on 14 Aug 2009 at 8:37

GoogleCodeExporter commented 9 years ago 
.iterator().next() is a extremely well-established java idiom, since it is the 
only way 
to get the first element of a Collection or a Set for at least a _decade_ now. 
So 
"readability" concerns are really moot, except for perhaps very 
recent/inexperienced 
java developers (who are clearly *not* the target audience of this library). 
Coming by 
2009 and introducing a longer version of something as common and which works 
perfectly 
would be a very strange decision for google collections.

Original comment by jim.andreou on 14 Aug 2009 at 10:08

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 17 Sep 2009 at 5:01

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 17 Sep 2009 at 5:45

GoogleCodeExporter commented 9 years ago

Original comment by kevin...@gmail.com on 17 Sep 2009 at 5:57

GoogleCodeExporter commented 9 years ago 
I don't think I ever made it clear:  I believe we *should* add an overload 
accepting a 
"T defaultValue" to each Itera*s method we currently have that accepts an 
Iterable<T>, 
returns a T, and throws an exception if no suitable T is found.  However many 
methods 
that is.

Original comment by kevinb@google.com on 16 Mar 2010 at 8:04

GoogleCodeExporter commented 9 years ago

Original comment by kevinb@google.com on 30 Jul 2010 at 3:50

GoogleCodeExporter commented 9 years ago 
Looks like we've done this get, getLast and getOnlyElement but just need to do 
it for find and we're done.

Original comment by kevinb@google.com on 30 Aug 2010 at 5:33

GoogleCodeExporter commented 9 years ago 
Wait, no, we never did the *first* part of this which is to add getFirst(Iter, 
T).

And forget what I said about find(), because that's tracked in bug 150. That's 
a little weird but that's how it is so let's just keep it that way.

Original comment by kevinb@google.com on 30 Aug 2010 at 5:39

GoogleCodeExporter commented 9 years ago

Original comment by boppenh...@google.com on 30 Aug 2010 at 5:59

GoogleCodeExporter commented 9 years ago 
Committed in r105.

Original comment by boppenh...@google.com on 4 Sep 2010 at 5:38

GoogleCodeExporter commented 9 years ago 
Please reconsider adding Itera*s.getFirst/getNext(Itera* i)

For Iterables, the following methods exist:
* public static <T> T get(Iterable<T> i, int p)
* public static <T> T get(Iterable<T> i, int p, T defVal)
* public static <T> T getFirst(Iterable<T> i, T defVal)
* public static <T> T getLast(Iterable<T> i)
* public static <T> T getLast(Iterable<T> i, T defVal)
* public static <T> T getOnlyElement(Iterable<T> i)
* public static <T> T getOnlyElement(Iterable<T> i, T defVal)

It really looks like something is missing.

My colleagues who just start Java keep bugging me about the fact that 
iterator().next() is just not understandable at first sight.

Original comment by ogregoire on 23 Nov 2010 at 2:38

GoogleCodeExporter commented 9 years ago 
Can we also add a checked version Iterables#getFirst that takes no default 
value and throws a NoSuchElementException?

It would work exactly like Iterables.getLast(T). 

I often write Iterables.getFirst(it, null) and then check for null.

Original comment by ori.schw...@gmail.com on 22 Dec 2011 at 8:43

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
It's already mentioned that you need to call - iterable.iterator().next()(which 
will throw NoSuchElementException) than Iterables.getFirst(it, null) and then 
check for null.

Original comment by ipremra...@gmail.com on 24 Dec 2011 at 10:01

GoogleCodeExporter commented 9 years ago 
How about returning an Optional on getFirst?

public static <T> Optional<T> getFirst(Iterable<T> i)

Original comment by mar...@aie.pl on 16 May 2013 at 8:09

GoogleCodeExporter commented 9 years ago 
We did that for FluentIterable.  I doubt it's worth going back and fixing 
Iterables now.  Use FluentIterable!

Original comment by kevinb@google.com on 22 May 2013 at 4:22

GoogleCodeExporter commented 9 years ago 
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:16

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:10