jamesbrowder / guava-libraries

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

Implement isAbsent() on Optional #734

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In the source code to the Optional datatype in revision 10.0, there is a //TODO 
for adding a isAbsent method call to the implementations. Please can this be 
implemented as I believe it conveys the intent better, i.e.,:

this:

if(reference.isAbsent()) {
  // do something
}

is clearer (and easier to maintain and less bug prone than):

if(!reference.isPresent()) {
  // do something
}

It is quite easy to forget to add in the ! condition.

Thank you.

-=david=-

Original issue reported on code.google.com by dharri...@gmail.com on 3 Oct 2011 at 12:00

GoogleCodeExporter commented 9 years ago
I agree, although or(T)/or(Supplier<T>) can be used, sometimes you want to 
handle the case differently, such as throwing an exception.

Original comment by dale.wij...@gmail.com on 3 Oct 2011 at 12:34

GoogleCodeExporter commented 9 years ago
It also fits when one is searching for something in a database (my use case).

If I search for something and it is not found, I am now using the Optional 
datatype that may or may not contain the found entity.

I don't think using or(<T>), or the or(Supplier<T>) would fit, since I don't 
want to return a default value (it wouldn't make sense to return a "default" 
entity since it will pretent the entity has been found whereas it hasn't.

I would just perfer to do if(reference.isAbsent()) handle it in some way, and 
as the previous comment mentions, I may want to throw an exception or perhaps 
do something else.

-=david=-

Original comment by dharri...@gmail.com on 3 Oct 2011 at 1:48

GoogleCodeExporter commented 9 years ago
I can't see a reason not to add this.  Patch welcome.

Original comment by kevinb@google.com on 5 Oct 2011 at 4:57

GoogleCodeExporter commented 9 years ago
Hi, 

Done. Please find attached a patch file.

-=david=-

Original comment by dharri...@gmail.com on 5 Oct 2011 at 10:03

GoogleCodeExporter commented 9 years ago
In the original review, we rejected this on the grounds that the most common 
if() statement is...

if (reference.isPresent()) {
  doSomething(reference.get());
}

...and that it seems far more likely to mistakenly write...

if (reference.isAbsent()) {
  doSomething(reference.get());
}

...than to mistakenly write...

if (!reference.isPresent()) {
  doSomething(reference.get());
}

We'll continue discussing.

Original comment by cpov...@google.com on 5 Oct 2011 at 8:13

GoogleCodeExporter commented 9 years ago
Hi,

Thanks for your comments, but I disagree. I think it is clearer to write 
reference.isAbsent() and deal with an outcome than write 
!reference.isPresent(), additionally I believe it would be very unlikely to 
call a reference.get() after writing isAbsent() as in your example (since one 
has explicitly written isAbsent thus putting firmly in the mind that there is 
no reference to get whilst writing code)

Having the isAbsent method (which is only a few additional lines of code to the 
Optional class), conveys the intent better that sometimes you expect the 
reference to not contain the value - therefore one writes code to handle it.

I further believe that it is easier to read (whilst scanning down a page full 
of code) which makes maintaining simplier IMHO. The number of times I've 
accidently left out the ! in the course of my programming career is something 
that, whilst embarrassing, does happen (and sometimes it's hard to notice the ! 
in code if you're scanning down a file...)

I would ask you to consider adding it please. There is nothing stopping people 
from using the !reference.isPresent() if they wish, but having a 
reference.isAbsent() just feels to me nicer.

Thank you.

-=david=-

Original comment by dharri...@gmail.com on 5 Oct 2011 at 8:50

GoogleCodeExporter commented 9 years ago
I agree with both "the exclamation mark is easily overlooked" and "'present' 
and 'absent' look too similar".

So what about adding the inverse method, but naming it `isMissing` instead? 
Sounds not as academic to me, but might solve the issue. (`isUnavailable` could 
basically work, too, but I think such methods are better namend without a 
negation in the name, so it would become `!isAvailable` and we already have 
`isPresent` …).

Original comment by j...@nwsnet.de on 6 Oct 2011 at 9:18

GoogleCodeExporter commented 9 years ago
We don't feel this is a clear enough advantage.

Original comment by fry@google.com on 16 Nov 2011 at 7:42

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:15

GoogleCodeExporter commented 9 years ago

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