• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Any advantage with this style?

 
Ranch Hand
Posts: 78
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Friends,

I have come across a method that expects an empty Map, returns the status of execution, if the execution was successful map will be populated with prices at various locations.

here's the actual method signature


Is there any benefit/disadvantage with this style?

I thought the following would be more natural,

the client has to check if the map is empty to see if the call was successful.

The first method is not extremely complicated to understand but I thought the second one is more cleaner.

I would appreciate any comments.

Thanks.
Cnu
[ June 09, 2005: Message edited by: cnu sri ]
 
author & internet detective
Posts: 41860
908
Eclipse IDE VI Editor Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Cnu,
That first style is confusing at it relies on a side effect to implement the functionality. While an empty map may be a valid case, there are a couple of other things you could to to check the status:
1) Return null - Better than relying on side effects or empty maps, but still requires and extra check.
2) Throw exception - This is the one I prefer.
3) Add isSuccessful() method to class that the client can call to see the status.
 
Bartender
Posts: 1205
22
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
If there's a possibility that creating the requested Map might fail, neither one of the above examples is great. Rather than returning a boolean value to show a failure, have the method throw an Exception. That's what the throw/try/catch/finally mechanism is for.



(Instead of Exception you might get more specific with either an existing Exception subclass that makes sense or a custom one.)

But that's iff something went wrong with creating the Map, or related processing. If it turns out that there just isn't any data for an otherwise valid request (give me a Map of names and id numbers for all our customers 65 or older), then I would have the method return a non-null Map with zero entries. Then use the Map isEmpty() or size() to determine how many key/value mappings were returned.

Ryan
[ June 09, 2005: Message edited by: Ryan McGuire ]
 
Ranch Hand
Posts: 1683
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Return null - Better than relying on side effects or empty maps, but still requires and extra check


I disagree. An empty map makes entire sense as the return type is Map, so why return a different type? I really dislike null being returned in a case like this.


Throw exception - This is the one I prefer


I disagree. An exception should only be thrown if something went wrong, eg database server has gone down. Note that it may be perfectly valid for no prices to be available, so why throw an exception?

Add isSuccessful() method to class that the client can call to see the status


I disagree once more . You should not put the onus on the developer of the client code to check for something going wrong. If something does go wrong, throw an exception. You get much more robust code this way.
 
Ranch Hand
Posts: 88
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Speaking as an Olde C++ Programmer (however, I *DID* get carded the other night!), this looks like Java code written by a cantankerous C++ programmer who says things like "Garbage Collection Is For Weenies." Conventional wisdom in C++ often says not to new/malloc inside a method unless you also delete (in which case you can't return a reference to the new stuff). Normally, the person who does a new is in charge of doing a delete, hence the method gets passed a reference to an already instanciated container that gets filled in by the method. Coulda been me. Before I "Saw The Beauty in Garbage (Collection)," that is.

I agree that



is what I'd like if I were using your method.

Mark
 
Jeanne Boyarsky
author & internet detective
Posts: 41860
908
Eclipse IDE VI Editor Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Roger,
I think we disagree because we interpreted "if the execution was successful map will be populated with prices at various locations" in different ways. I took it to mean that the map gets filled (or happens to have no values) if all goes well.

I disagree. An empty map makes entire sense as the return type is Map, so why return a different type? I really dislike null being returned in a case like this.


Me too. However, no prices (an empty map) is different from an error. Null isn't a great way to handle the error/exception either, but is better than relying on side effects. If you return an empty map for errors too, the caller has no way of knowing the difference.

I disagree. An exception should only be thrown if something went wrong, eg database server has gone down. Note that it may be perfectly valid for no prices to be available, so why throw an exception?


I interpretted it to mean that something had gone wrong. If no prices happen to be available, I agree you shouldn't throw an exception.

I disagree once more . You should not put the onus on the developer of the client code to check for something going wrong. If something does go wrong, throw an exception. You get much more robust code this way.


I don't particularly like this approach either. Throwing an exception is cleaner. However, I like this better than the side effect approach originally posted.

Basically, I did say that I favor the Exception approach that Ryan and Mark posted. I just did a more roundabout way
 
Pavan Kumar
Ranch Hand
Posts: 78
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks a lot guys. I certianly have learned a trick.
I very much appreciate your inputs. I apologise if my original post was a bit misleading.
is very cool.

Mark's explanation is also very cool.

Regards,
Cnu.
 
Sheriff
Posts: 67746
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

disagree. An empty map makes entire sense as the return type is Map, so why return a different type? I really dislike null being returned in a case like this.



And I completely disagree with this. Why go through the overhead of creating and discarding an empty Map when a null serves the purpose quite nicely?

I love null being returned in cases like this.

As for an exception, I would only throw an exception in an exceptional case. If it's an expected usage case that the Map cannot be populated I would not use an exception to indicate that condition. I would return the null.

Never send an exception to do an if statement's job.
[ June 09, 2005: Message edited by: Bear Bibeault ]
 
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Bear Bibeault:
And I completely disagree with this. Why go through the overhead of creating and discarding an empty Map when a null serves the purpose quite nicely?

I love null being returned in cases like this.



It depends. If the client just can work with the empty Map, it can be a quite convenient way to implement this. (It's an instance of the Null Object pattern, by the way.)


As for an exception, I would only throw an exception in an exceptional case. If it's an expected usage case that the Map cannot be populated I would not use an exception to indicate that condition. I would return the null.

Never send an exception to do an if statement's job.



I agree with that.

I dislike using null for any kind of special conditions, too, though. I very much prefer



over



for example.
 
Roger Chung-Wee
Ranch Hand
Posts: 1683
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

And I completely disagree with this. Why go through the overhead of creating and discarding an empty Map when a null serves the purpose quite nicely?

I love null being returned in cases like this.


I don't think that efficiency should ever be a consideration here. What we must do is produce code which adheres to the contract, which says that a Map should be returned.

Furthermore, it can be awkward if null is returned. First, the client has to check for null. If the map is not null, does this guarantee that the map is not empty? It does not, so a second check is required for this.

How much easier it is to simply invoke something like map.isEmpty() to check for an empty map if the client knows that a map will always be returned.
 
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
On the extremely rare (IMO) occasions that the performance overhead of creating an empty Map might actually matter - it's easy to simply return a reference to a single immutable immutable Map instance which you reuse for this putpose. Like, say, Collections.EMPTY_MAP. Type-safe views of this are available via Collections.emptyMap() in 1.5.

I'm strongly in the "don't return null if you can help it" camp. At least, any time the object being returned is a collection, map, array, or anything else that returns a variable number of things. The user has to code for the possibility that the size is zero anyway - why make them check for null too?

See also the Book of Joshua, Item 27.
 
Bear Bibeault
Sheriff
Posts: 67746
173
Mac Mac OS X IntelliJ IDE jQuery TypeScript Java iOS
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator



I very much prefer

if (priceProvider.hasPricesFor(product)) {



This is a pattern I frequently use and would also prefer.
 
Ranch Hand
Posts: 323
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
i've done the null trick, and i've come to dislike it. for me, it's not so much that it needs an extra test, but that it breaks some kind of semantic; methods should return some-kind-of-thing, or else throw an exception — that's what declaring a return type and using a "throws" statement mean. null is neither a thing nor an exception, it's something unexpected and unexplained.

and it's not just a matter of type checking, to me, either. it's more on a level of functional thinking; if a function/method can return all these kinds of objects and null, then the null represents a discontinuity in the function's domain. it's untidy.

oddly enough, i tend to return null (None) more when i code Python. for some weird reason, it seems less dissonant there. probably something to do with dynamic vs. static typing; the overhead of mentally handling it might be less in a dynamic language. but even then, i still think it's a bad idea; if a function/method needs to return something that can't be fit into its regular domain, it should throw an exception — that's the accepted, general out-of-band signalling method these days. in-band signalling by returning an out-of-domain value is just messy, it confuses different levels of logic. ("is this a valid return" versus "was anything returned at all", i think it is. they should be kept separate.)
 
Pavan Kumar
Ranch Hand
Posts: 78
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks very much once again for your valuable opinions and time.

I just placed an order for Effective Java .

Best Regards,
Cnu.
 
Ranch Hand
Posts: 354
Eclipse IDE Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I would vote for returning NULL. Client would anyway check for null before checking whether there are any elements inside.
 
Ilja Preuss
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Jack Ryan:
I would vote for returning NULL. Client would anyway check for null before checking whether there are any elements inside.



Why would he do that if he knows that the method never returns null?
 
(instanceof Sidekick)
Posts: 8791
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
We had a boiler-plate technique from our vendor that looked like this:

If they had chosen return empty vector instead of null in their service designs, both of those tests could go away. Of course the second test was always unnecessary. I think they were getting paid by the pound some days. Our new services do not return null, but now we have a mix and you almost have to go read the called service to see if you can trust it.

What if the method returns a single object or null, rather than a collection? NullObject is also a neat pattern. Intead of returning null return a special object that has the right behavior for a "no data" situation.
[ June 11, 2005: Message edited by: Stan James ]
 
Abhinav Srivastava
Ranch Hand
Posts: 354
Eclipse IDE Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Ilja Preuss:


Why would he do that if he knows that the method never returns null?



I should have said - "should anyway"

How does he know that the method never returns null? By looking at the code,
by asking the person who wrote it. Passing new xMap() as argument is not ensuring that. And if he 'knows' the method never returns null, why pass new xMap()!
But then "fair weather" programming has its own way!
 
Jim Yingst
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
["Jack Ryan"]: How does he know that the method never returns null?

My first choice would be to look at the API. If the docs say that the method may return null in certain circumstances, then certainly I will test for null. If the docs say the method "returns a Foo", and they don't say anything about returning a null, then I figure the method is supposed to return a Foo, or throw an exception. It's possible the method author was incompetent of course, but that's not my first assumption when I confront a new API. It's also possible that the method will throw a ClassCastException or some other random RuntimeException for no reason. I'm not going to try to put catch blocks around every method I call anticipating all possible undocumented bugs of that code. Instead I assume that the method behaves according to its API, and code accordingly. Then write a lot of tests to verify that the code works as intended. If it turns out the method really does return null in some circumstances, I'd consider it a bug in the method. Of course I can insert a workaround, but I'm also going to be trusting that jar file a lot less. And working on either getting it fixed, or getting it out of my project entirely, if possible.

Of course, there are many 3rd party APIs that don't really have decent JavaDoc, but they do have source code available, and often they have sample usage documentation. In those cases I can usually determine for myself whether a method may return null or not. If not, I have to seriously question why I would use such opaque undocumented code in the first place. If necessary, I can put extra error checking around such code, sure. (Presuming it performs some worthwhile service that makes it worth tolerating such bad manners.)

By looking at the code, by asking the person who wrote it.

Also possible. If it's locally developed, that also means I'll be able to have the author beaten if it turns out they're returning undocumented nulls.

Passing new xMap() as argument is not ensuring that. And if he
'knows' the method never returns null, why pass new xMap()!


Sorry, I'm not sure what you're saying here. Is this a reference to very first getPrices() signature in the first post of this thread? No one in this thread has ever advocated that technique. Seems to be a dead issue - no one likes it. Are you reading the same thread I am?

But then "fair weather" programming has its own way!

A friend of mine referred to this strategy as "coding for success".

I prefer to keep the code as clean and readable as possible. Unnecessary null checks obscure the "real" code. More importantly, they often make it harder to find an error when it does occur. If you do put in a null check for something that has no good reason to be null - please at least log an error if a null is detected. Many coders simply hide the error by doing nothing. Which often just makes it more difficult to track down the real source of a problem.

Of course in many applications, there's an end user who must be shielded from unsightly stack traces. In such situations I typically find the layer that's just before a user would see such an error, and insert a catch Exception (or sometimes even catch Throwable(!)) to intercept random events and log them somewhere, and show the user some innocuous message like "An internal error has occurred. If the problem persists, please notify us as [somelink]." Then I spend the remainder of development analyzing and testing to find ways that such an error could occur, and prevent them at the source. Depending how much the client/employer values speed over robustness, I can provide a prototype very quickly, a "good enough" version fairly quickly, or a more bulletproof version after more time elapses. Or all the above. In most cases in my experience, the requirements will change shortly after the prototype is demonstrated anyway, and so there's not much use for premature bulletproofing.
[ June 13, 2005: Message edited by: Jim Yingst ]
 
Ilja Preuss
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Jack Ryan:
How does he know that the method never returns null? By looking at the code,
by asking the person who wrote it.



Yes. Or by actually being the author of it, too...

But then "fair weather" programming has its own way!



If I understand you correctly, that's actually the style I prefer, yes. It makes the code so much simpler, and thereby more stable.
 
Abhinav Srivastava
Ranch Hand
Posts: 354
Eclipse IDE Oracle Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
To be fair and frank -
it boils down to how contracts are defined. If you are using 3rd party libraries, you have got good documntation, samples, forums ... etc.
In house developments generally do not witness so much discipline. (I am only 3.5 years in job!). With frequently changing requirements the code any way keeps going unstable every other day. And thats where the tendency to make the code 'bullet-proof' sneaks in. After all who likes a NullPointerException but that is besides the point.
null is fairly well used as an indicator for the absence of something. So can be an empty object or even an exception.
isEmpty() is however not always available. null is.

salute.
[ June 14, 2005: Message edited by: Jack Ryan ]
 
Ilja Preuss
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Jack Ryan:
In house developments generally do not witness so much discipline.



That's a matter of choice, in my not so humble opinion. I typically prefer to write better code than that I get from the outside.

With frequently changing requirements the code any way keeps going unstable every other day.



Only if you are lacking techniques to keep the code clean and stable.

I am working on a legacy system that is now under development for over seven years and has approximately a half million lines of code. Since I joined the team four three years ago, we have introduced practices like pair programming, unit testing and merciless refactoring, and the stability of the code has improved significantly.


And thats where the tendency to make the code 'bullet-proof' sneaks in. After all who likes a NullPointerException but that is besides the point.



The error is that a null reference occured where there shouldn't have one occured. This needs to be analyzed, fixed and learned from. "Checking for null" typically will just hide the true error, and thereby hold us from finding the true problem and learning from the incident.

null is fairly well used as an indicator for the absence of something. So can be an empty object or even an exception.



All of those can be used. The interesting thing to discuss is which of those techniques is more likely to lead to systems which are robust and easy to maintain and extend.


isEmpty() is however not always available. null is.



It almost always *can be made* available, can't it?
 
Roger Chung-Wee
Ranch Hand
Posts: 1683
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

it boils down to how contracts are defined. If you are using 3rd party libraries, you have got good documntation, samples, forums ... etc.
In house developments generally do not witness so much discipline.


It should be exactly the opposite. An inhouse development means that you have complete control over the quality of the code and documentation. And note that the source code of third-party products may not even be available for you to inspect.

With frequently changing requirements the code any way keeps going unstable every other day. And thats where the tendency to make the code 'bullet-proof' sneaks in.


Surely the code which is uploaded into your repository is tested? If you download code from the repository which is unstable, then you have problems. You need to deal with these problems rather than curing the symptoms, such as checking for null when null should never be returned.
 
Ryan McGuire
Bartender
Posts: 1205
22
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Bear Bibeault:


And I completely disagree with this [returning a non-null but empty Map if there's no data]. Why go through the overhead of creating and discarding an empty Map when a null serves the purpose quite nicely?

I love null being returned in cases like this.



And I completely disagree with this. I would rather NOT have to do anything different in the zero records case. I don't want to have a separate if (myMap != null) {...} block to treat the special case. Once I call getPrices() (and not get and Expception), I want to go straight into my for (Iterator i = myMap.keySet().iterator(); i.hasNext(); ) {...} block without having to worry about a NullPointerException.

The extra if (xyz==null) blocks, both in getPrices() and the calling code pretty much uses up the optimization of not allocating the empty Map object.

HOWEVER...
Since there seem to be two (almost ) equally valid schools of thought on the subject, be sure to javadoc your method so that client objects know what it'll return if there is no data.

Philosophical discussion:
Zero is just another number, so it shouldn't be treated differently. I don't see why people put up this big conceptual wall between 0 and [1,2,3,...].

Ryan
[ June 15, 2005: Message edited by: Ryan McGuire ]
 
Ilja Preuss
author
Posts: 14112
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Ryan McGuire:

HOWEVER...
Since there seem to be two (almost ) equally valid schools of thought on the subject, be sure to javadoc your method so that client objects know what it'll return if there is no data.



Or even better, document it writing a unit test!
 
They weren't very bright, but they were very, very big. Ad contrast:
a bit of art, as a gift, that will fit in a stocking
https://gardener-gift.com
reply
    Bookmark Topic Watch Topic
  • New Topic