search

MedwynLiang / google-collections

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

Preconditions.check{Element,Position}Index return values #240

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
From david@haubs.net,

"I noticed that the CheckNotNull methods returned the object reference, but 
the checkElementIndex and checkPositionIndex methods did not return the 
index.  Returning a value is nice because it means that the function can be 
used inline, without forcing it to be a separate statement."

Seems harmless to me.

Original issue reported on code.google.com by kevin...@gmail.com on 17 Sep 2009 at 7:27

GoogleCodeExporter commented 9 years ago 
I've realized that it may be important to preserve binary compatibility 
starting with 
1.0-final, and this is a change that is only source-compatible but not binary-
compatible, so we need to either do it now or never.

Original comment by kevin...@gmail.com on 18 Sep 2009 at 6:50

GoogleCodeExporter commented 9 years ago 
I made the change, and then reviewed all of our com.google.common libraries to 
see in 
which places I would want to change the code to make use of the return value.  
I 
didn't find a single one.  In too many cases, it just made the code harder to 
understand.  So now I am vacillating over whether it's a good idea to even 
return the 
value in the first place.  Do you really think you would use this return value 
to good 
effect often, or did your suggestion come more from hypotheticals?

Original comment by kevin...@gmail.com on 21 Sep 2009 at 8:41

GoogleCodeExporter commented 9 years ago 
One place where I could see this being useful is if you want to provide the 
check on
a variable that is being passed to the constructor of a superclass.  eg:

private static int staticSize;

public SubClass(int index) {
  super(Preconditions.checkElementIndex index, staticSize);
}

if you didn't have the return variable then you would have to invoke the 
superclass
constructor before you validated whether or not the variable was appropriate.

Original comment by Alex.Fie...@gmail.com on 21 Sep 2009 at 10:27

GoogleCodeExporter commented 9 years ago 
I would have to agree with Alex -- the return idiom can make things formerly
impossible be possible and be incredibly useful in other cases, all while 
making the
API only differentially harder to understand.  Clients can either use the return
value or ignore it.  As long as this is documented, I don't see a problem with 
this.

Original comment by russ.mil...@gmail.com on 22 Sep 2009 at 12:06

GoogleCodeExporter commented 9 years ago 
I'm not sure they make anything impossible possible, but I'm leaning toward 
making
the change in any case.

Original comment by kevin...@gmail.com on 22 Sep 2009 at 1:45

GoogleCodeExporter commented 9 years ago 
fixed for next rc.

Original comment by kevin...@gmail.com on 22 Sep 2009 at 8:22