• 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

How to deal with illegal arguments

 
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Besides asking for specifics, I would probably choose to throw an IllegalArgumentException instead with a message that the argument should have at least two elements -- it's a step further back from violating the Principle of Least Surprise than returning a value that's not actually a part of the list.
@Campbell - JavaDocs are a poor substitute


Have to disagree there. Documentation, in this case, is essential. In the absence of documentation, I would agree that POLS is probably the way to go.

Winston

[Edit] Split from here, as we geeks kind of veered off topic.
 
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:Actually, they are

I stand corrected and bow to your logic.

Edit: **** WORD OF CAUTION if you are going to attempt to follow this thread ****

What follows is a two-pronged discussion - one discussion is mostly about design choices around parameters and return values,
while another concurrent but slightly divergent discussion on the validity of "Integer.MIN_VALUE is the identity of max"

Note that the "Integer.MIN_VALUE" discussion ends with me making a similar "bow of failure" at the end so take all my responses leading up to that "bow" as a misguided attempt to refute an established fact. If you are so inclined, you can follow the thread to the end to see how it turns out for me in my pig-headed best.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:Have to disagree there. Documentation, in this case, is essential. In the absence of documentation, I would agree that POLS is probably the way to go.


In this case, documentation would be useful, not necessarily essential. POLS wins over documentation any day in my book -- you can document it all day if you want, if it's still surprising to me as a user of the method/class, documentation isn't worth a darn keystroke. A poor design choice is a poor design choice, no matter how much you document it. Aren't you one of those who rage against java.util.Date?
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Aren't you one of those who rage against java.util.Date?


Actually, I have more problems with Calendar than Date, but I take your point.

I guess the question here is: Shouldn't a function that returns a "2nd best" define its action in the case where there is no 2nd best? If it doesn't, that's bad design by omission (or laziness).

I have no problem with it throwing an Exception, but we're used to defaults; why not here? You could even give it a nice name like NO_SECOND BEST. About the only thing I wouldn't do is have it return null because, to me, that definitely violates POLS.

Winston

PS: In theory, you're also supposed to document all Exceptions that a method throws of its own accord.
 
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

William Koch wrote:The assignment assumed that there were at least 2 Integer elements in the list.



Then I would throw an unchecked exception (such as IllegalArgumentException) in the case of there not being at least 2 elements, and document this behavior.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:

Winston Gutkowski wrote:Have to disagree there. Documentation, in this case, is essential. In the absence of documentation, I would agree that POLS is probably the way to go.


In this case, documentation would be useful, not necessarily essential. POLS wins over documentation any day in my book -- you can document it all day if you want, if it's still surprising to me as a user of the method/class, documentation isn't worth a darn keystroke.



But if you read the documentation--and why wouldn't you?--then as long as the thing behaves as documented, there's no cause for surprise. In particular, if the documentation says, "If you do X, the behavior is undefined," then you as a user have no excuse for being surprised at any behavior if you do X. Undefined behavior for invalid input is perfectly valid in some cases.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:but we're used to defaults; why not here?

Because saying that there is a 2nd value in a collection that doesn't have one make no sense to me -- there is no sensible "default" value. It's like asking me, "Who is your second child?" and I either had only one child or no children; it makes no sense for me to say "Well, I don't have one but let's just say his name is Bob," does it?
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Jeff Verdegan wrote:Undefined behavior for invalid input is perfectly valid in some cases.


That's moot but let's not lose track of the current case. The OP already clarified the requirement that the method expects a List that has at least 2 elements in it. Therefore, an IllegalArgumentException rather than some "default" value or leaving the behavior as undefined is a better design choice, IMO. (Edit: I see we're already on the same page here )
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:

Jeff Verdegan wrote:Undefined behavior for invalid input is perfectly valid in some cases.


That's moot



No, it's directly relevant to the OP's issue and to your previous comments.

The OP already clarified the requirement that the method expects a List that has at least 2 elements in it.



Yes, I saw that after I posted the above. Hence my reply to the OP which quoted that assumption and advised him to throw an unchecked exception.

Therefore, an IllegalArgumentException rather than some "default" value or leaving the behavior as undefined is a better design choice, IMO.



Yup.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:

Winston Gutkowski wrote:but we're used to defaults; why not here?

Because saying that there is a 2nd value in a collection that doesn't have one make no sense to me -- there is no sensible "default" value.



In some cases, the identity element (which would be Integer.MIN_VALUE here) is an appropriate output for max(empty set). So by extension, it could be an appropriate value for second_max(set with < 2 elements). Not in this case, of course, since we've already established the precondition that there be >= 2 elements.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Because saying that there is a 2nd value in a collection that doesn't have one make no sense to me -- there is no sensible "default" value. It's like asking me, "Who is your second child?" and I either had only one child or no children; it makes no sense for me to say "Well, I don't have one but let's just say his name is Bob," does it?


The analogy is inaccurate. One of the requirements of good design is that ALL situations are accounted for. If I have a method called
secondChildsName()
then I need to provide for the fact that the method may be nonsensical to the respondant, and either
(a) throw an Exception.
(b) return a value that indicates that the question was nonsensical.
In either case I need to document what happens in such a situation.

Winston
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Jeff Verdegan wrote:In some cases, the identity element (which would be Integer.MIN_VALUE here) is an appropriate output for max(empty set)


I don't know if I buy that one either. So are you saying that max({Integer.MIN_VALUE}).equals(max({})) is true? The logic eludes me...
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:The analogy is inaccurate... b)return a value that indicates that the question was nonsensical.

Aren't you actually validating my analogy here? Isn't this just like shrugging when asked a non-sensical question? Also, a default value is a valid value that you use in the absence of any other valid value. What you're talking about is an ERROR FLAG value, not a default.
 
Marshal
Posts: 28226
95
Eclipse IDE Firefox Browser MySQL Database
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Here's the logic, then.

What's the sum of an empty array of numbers? It's necessary to consider what the properties of such a value would be. For example you would expect that sum(A) + sum(B) = sum(A union B) for all lists A and B. So that means that sum({}) + sum(B) = sum(B), in other words the sum of an empty array must be 0. And that happens because the + operator has an identity element, named 0, such that 0 + x = x for all values of x.

Now, what's the product of an empty array of numbers? Going through the same logic, you expect that product({}) * product(B) = product(B), and thus the product of an empty array of numbers must be 1. Again, 1 is the identity element of the * operator because 1 * x = x for all values of x.

So finally, what's the maximum of an empty array of numbers? By now we know we're looking for the identity element of the "max" operator. And since Integer.MIN_VALUE max x = x for all values of x (in the range of Java ints, that is) it follows that Integer.MIN_VALUE is the answer. (In real life where you're working with arbitrary integers, the answer is minus infinity.)
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:In this case, documentation would be useful, not necessarily essential. POLS wins over documentation any day in my book -- you can document it all day if you want, if it's still surprising to me as a user of the method/class, documentation isn't worth a darn keystroke.


So how does your understanding of POLS work in the case of an indexOf() method, Junilu? There you have the case of a method which is highly likely to return a value that indicates "not found". In such a case you absolutely should NOT throw an Exception since, as we both know, Exceptions are used for "exceptional" behaviour; and yet you cannot return a valid index either. Furthermore, you don't have a single identity value (as you do with max()).

I've actually heard it argued that the best value to return is Integer.MIN_VALUE, since it's the least likely to occur (at least not quickly ), and allows for the fact that in some situations an "index" could be negative; but in general I stick with -1, because that's what the rest of the world does.

Winston
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Paul Clapham wrote:Here's the logic, then. ...
So finally, what's the maximum of an empty array of numbers? By now we know we're looking for the identity element of the "max" operator. And since Integer.MIN_VALUE max x = x for all values of x (in the range of Java ints, that is) it follows that Integer.MIN_VALUE is the answer. (In real life where you're working with arbitrary integers, the answer is minus infinity.)



That seems like quite a leap. Following your previous logic: max(A, B) ==> max(A union B) ==> max({} union B) ==> ?what is here to get to the next step? ==> max({Integer.MIN_VALUE} union B)

Where did Integer.MIN_VALUE come from all of a sudden? There is no equivalence between max({}) and max({Integer.MIN_VALUE}) -- I contend that max({}) is UNDEFINED, not Integer.MIN_VALUE. If you want to say for purposes of calculation, you can consider max({}) as Integer.MIN_VALUE, then that's fine but logically speaking, if max({}) were to return a set, it would return a null set, not a set with one element (Integer.MIN_VALUE).
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Aren't you actually validating my analogy here? Isn't this just like shrugging when asked a non-sensical question? Also, a default value is a valid value that you use in the absence of any other valid value. What you're talking about is an ERROR FLAG value, not a default.


Whatever I'm talking about, it's perfectly renderable with:
public static final String NO_SECOND_CHILD = new String("*No second child*");
which, unless you live in a world where a real name can have multiple words containing asterisks, is likely to satisfiy both '==' and equals() checks.

Or do you really want a situation where a whole pile of methods need to be executed inside try...catch blocks simply to satisfy some notion of POLS?

Winston
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:I contend that max({}) is UNDEFINED



I used to think that too. Paul was kind enough to set me straight, though the idea still felt squicky for a while.

Mathematically speaking max({empty set of java ints}) is Integer.MIN_VALUE. That doesn't mean it's always the correct result for a Java max() method that's handed empty input (and, as we've already laid out, not in this case), but the principle is mathematically valid, and, I gather, pretty commonly used.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Jeff Verdegan wrote:I used to think that too. Paul was kind enough to set me straight, though the idea still felt squicky for a while.


Good old Maths. Defines the undefinable.

Winston
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:

Junilu Lacar wrote:In this case, documentation would be useful, not necessarily essential. POLS wins over documentation any day in my book -- you can document it all day if you want, if it's still surprising to me as a user of the method/class, documentation isn't worth a darn keystroke.


So how does your understanding of POLS work in the case of an indexOf() method, Junilu? There you have the case of a method which is highly likely to return a value that indicates "not found". In such a case you absolutely should NOT throw an Exception since, as we both know, Exceptions are used for "exceptional" behaviour; and yet you cannot return a valid index either. Furthermore, you don't have a single identity value (as you do with max()).

I've actually heard it argued that the best value to return is Integer.MIN_VALUE, since it's the least likely to occur (at least not quickly ), and allows for the fact that in some situations an "index" could be negative; but in general I stick with -1, because that's what the rest of the world does.

Winston



Haven't stumped me yet

indexOf() is a slightly different beast. The -1 return value is a FLAG value, not a default. And I would have been fine with it returning Integer.MIN_VALUE too, if that's what the original designer would have chosen. I still don't buy that Integer.MIN_VALUE is the identity value for max({}) for reasons I already posted. In the case of throwing a RuntimeException like IllegalArgumentException, you are using it to signal a violation of a usage contract, with the intent of informing the programmer that he/she messed up in the usage of the class or did not validate the input properly. I agree, throwing an exception probably wouldn't be a good choice for indexOf() but -1 is not as objectionable in this case because it's outside the range of valid values (valid indices are 0 to length-1). In the case of secondBiggest(), Integer.MIN_VALUE is NOT outside the range of valid values because you can have a list that is {Integer.MIN_VALUE, Integer.MIN_VALUE} --

So how would secondBiggest({Integer.MIN_VALUE, Integer.MIN_VALUE}) being equal to secondBiggest({}) make any sense to you? The logic there simply does not compute.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:
So how would secondBiggest({Integer.MIN_VALUE, Integer.MIN_VALUE}) being equal to secondBiggest({}) make any sense to you? The logic there simply does not compute.



The same way max({}) is equal to max({negative infinity}) on the real numbers.
 
Paul Clapham
Marshal
Posts: 28226
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:That seems like quite a leap. Following your previous logic: max(A, B) ==> max(A union B) ==> max({} union B) ==> ?what is here to get to the next step? ==> max({Integer.MIN_VALUE} union B)

Where did Integer.MIN_VALUE come from all of a sudden? There is no equivalence between max({}) and max({Integer.MIN_VALUE}) -- I contend that max({}) is UNDEFINED, not Integer.MIN_VALUE. If you want to say for purposes of calculation, you can consider max({}) as Integer.MIN_VALUE, then that's fine but logically speaking, if max({}) were to return a set, it would return a null set, not a set with one element (Integer.MIN_VALUE).



You're in denial here and instead of examining the argument, you're just throwing out irrelevancies.

Where did Integer.MIN_VALUE come from? It came from examining the "max" operator and observing what must be its identity element. There's no magic or underhanded business going on, just ordinary observation of how things work.

And then you go on about "if max({}) were to return a set". But it doesn't return a set. It returns a single number. There's no "logically speaking" about that, you're just inventing things which don't exist. And you say "there is no equivalence between max({}) and max({Integer.MIN_VALUE})" -- I don't know why you think this is important, but there is such an equivalence, and it's just the same as the equivalence between sum({}) and sum({0}).

However it's certainly possible for you to contend that max({}) should be UNDEFINED, but then you also have to accept that sum({}) is UNDEFINED. That's a perfectly acceptable way to define how operators work on empty sets.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Haven't stumped me yet


I'm glad. And hope you're not taking all this ganging up personally. A bit of healthy debate never did anyone any harm - and I do hear what you're saying; just not sure you can always afford to be purist.

How's that for a bit of hypocrisy?

Winston
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:In the case of secondBiggest(), Integer.MIN_VALUE is NOT outside the range of valid values because you can have a list that is {Integer.MIN_VALUE, Integer.MIN_VALUE}
So how would secondBiggest({Integer.MIN_VALUE, Integer.MIN_VALUE}) being equal to secondBiggest({}) make any sense to you? The logic there simply does not compute.


Fraid I'm not the mathematician that Paul is, but it makes perfect sense to me that such a method might have a flag value; and even more that documentation is absolutely essential for it - including the possible situations in which the flag value is returned.

Imagine that this method needed to be run on a billion such arrays. Are you happy to have your program plough through 999 million of them and then throw an Exception? Not I.

Winston
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
No, not taking it as a ganging up against me thing at all It's all good -- polite but spirited discussion around differences of opinion is good for the soul and gray matter. My Peacemakers are still securely in the safe. And I'll be the first to admit it when you get through my thick skull
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:Imagine that this method needed to be run on a billion such arrays. Are you happy to have your program plough through 999 million of them and then throw an Exception? Not I.


Shirley, you exaggerate but I would do what I tell my guys to do to avoid RTEs because they violated a contract:
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:...but I would do what I tell my guys to do to avoid RTEs because they violated a contract:


Hmmm, A contract, eh? And where would that be written?

Just for comparison purposes, my solution to the method would probably be:Not optimal, but fairly generic.

Winston
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:
Just for comparison purposes, my solution to the method would probably be:Not optimal, but fairly generic.


It's good enough for me, dude. The only WTF I would have during a code review of this would be about the smilie in the comment
 
Marshal
Posts: 79239
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I would suggest if (list == null) throw new NullPointerException();
You probably do not have a contract that you throw no Exceptions. You have a precondition that nulls are not permissible, so enforce it by throwing that Exception. And put a @throws tag in your /** comment */.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Campbell Ritchie wrote:I would suggest if (list == null) throw new NullPointerException();


At the risk of sparking another debate, should you throw NPE or IllegalArgumentException? I absolutely agree that in that case you should throw some sort of Exception, and also that it's different from supplying a list that's too short. I have a general aversion to throwing NPE's myself, but I think in this case you may be right.
I also reckon you should throw it explicity - probably with a message - rather then relying on
if (list.size() < 2)
to do it for you.

You probably do not have a contract that you throw no Exceptions. You have a precondition that nulls are not permissible, so enforce it by throwing that Exception. And put a @throws tag in your /** comment */.


Absolutely.

Winston
 
Campbell Ritchie
Marshal
Posts: 79239
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:… should you throw NPE or IllegalArgumentException? …

Now that really would start a fight
I think it is NPE if you pass a null and IAE if you pass a “real” argument which has the wrong value. So I think we would agree in this instance.
 
Campbell Ritchie
Marshal
Posts: 79239
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
If I were throwing an Exception for a List too small, it would be an IAE. Lists which are too small can still be used elsewhere, but nulls are nasty hazardous things which come back and bite you with NPEs elsewhere if you let them loose.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks, Campbell. My rules of thumb in code reviews:

1. Always look for a test -- if a test doesn't cover a certain behavior, the programmer missed it or is falling away from doing Test-Driven Development - the tests should specify what the expected behavior is for null and/or invalid input values.

2. Let NullPointerExceptions get thrown as they will -- there should be a test that covers that. There are ways to avoid having to deal with NPE besides precondition validation: NULL Object pattern is one approach; a related approach is to always pass a List, even if it's an empty one, just never pass in NULL. I believe this is in Josh Bloch's book(?).

3. Violations of public usage contract, which includes input that is out of the range of valid values, should be tested with the expectation that an IllegalArgumentException will be thrown.

4. Make sure the JavaDocs reflect the behavior -- no, I'm not totally against JavaDocs but they are, as I have said, a poor stand-in for good design choices. They also can be unclear, incomplete, or out of date, depending on how much discipline and care the programmers have in writing and updating them, so they are nice to have but I don't lean on them.

As my final note on the discussion about the identity element of max() - after reading the wikipedia entry on the Empty Set, I understand how negative infinity is the identity element for the maximum operator. However, to translate that to "Integer.MIN_VALUE is the identity element of max()" (in the context of collections, where an empty collection can be considered) is not mathematically or logically correct and we should not try to bolster that assertion with a mathematical proof because that would be misleading at best.

I think that even the creators of Java had some idea of this (or maybe I'm making a stretch here a little bit too) because there is, in fact, a constant defined in java.lang.Double for NEGATIVE_INFINITY. The best alignment with the mathematics would be to say that "Double.NEGATIVE_INFINITY is the identity element of max()" but that's only good for collections of Double.

For computation purposes, I can accept a convention that says "For a collection of Integers, we will use Integer.MIN_VALUE to represent negative infinity, since there is no NEGATIVE_INFINITY constant for Integer." This does require additional checks when you code for it but oh well, what can you do, right? And by "additional check" I mean: if (set/list contains Integer.MIN_VALUE only) then max actually IS Integer.MIN_VALUE, otherwise Integer.MIN_VALUE means negative infinity. You could add another constraint to say that "Integer.MIN_VALUE will NEVER be a valid element of a set/list" then you can do away with the additional checks and Integer.MIN_VALUE is as good as NEGATIVE_INFINITY for Integers.

EDIT: in the context of discrete integer values A and B, where the empty set will not be considered, I can buy that Integer.MIN_VALUE is the identity element for max(A, B).

Are we all good with this? I am.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:Are we all good with this? I am.


'Fraid not. Sorry to rain on your beerfest, but I totally disagree with point 2.

I was taught a long time ago (and in a different language) that errors should be thrown early and LOUD; and simply "let[ting] NullPointerException's get thrown as they will" violates that principle in spades, resulting in it becoming an effect rather than a cause.

How many times have you run into a monumental JBoss or Servlet container stacktrace containing piles of "no source available" lines, simply because some nit failed to check that a value he was passing 17 layers back wasn't null? Fraid it's happened to me too many times to go along with the "just let it happen" paradigm.

And if you check out Item 38 in EJv2, I think you'll find that Josh Bloch agrees with me.
I have a feeling you may be confusing it with his advice that you should always return empty Lists or arrays rather than null.

And just one last point: While I have an in-built aversion to throwing NPEs myself, if I do, I always supply a message.

Winston
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:I was taught a long time ago (and in a different language) that errors should be thrown early and LOUD; and simply "let[ting] NullPointerException's get thrown as they will" violates that principle in spades, resulting in it becoming an effect rather than a cause.


It's absolutely sunshine where I'm standing on this Yours is actually in line with our practice because we do TDD and NPEs are thrown LOUDly and early as we code and evolve our designs. Our TDD-driven -- is that redundant, or what? -- designs usually end up taking care of the possibilities of NPEs getting thrown unexpectedly and we absolutely do not expect to see NPEs at all when we get to production. For me, an NPE in production is a bad thing and a big red flag that there's a programming error and/or a test missed somewhere along the line. I probably should have said "Let NPEs get thrown as they will as we test-drive the code and evolve the design, not in production" to avoid your indignant and virulent objection
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:I think that even the creators of Java had some idea of this (or maybe I'm making a stretch here a little bit too) because there is, in fact, a constant defined in java.lang.Double for NEGATIVE_INFINITY. The best alignment with the mathematics would be to say that "Double.NEGATIVE_INFINITY is the identity element of max()" but that's only good for collections of Double.


Hmmm, so maybe the design flaw was not in returning a flag value, but in returning an int, rather than an Integer.
If it had been the latter, we could have returned a constant (perhaps equal() to Integer.MIN_VALUE) that was unambiguous in its meaning.

Winston
 
Campbell Ritchie
Marshal
Posts: 79239
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Agree with Winston that the earlier you find out about a null value the better. An empty set/list/array is obviously far better than a null, provided we can agree on some sort of response to it. The problem with MIN_VALUE, NEGATIVE_INFINITY etc., is that these are potential real values. Agree about documentation comments. Writing bad documentation is nearly as bad as writing bad code.

I don’t think we are disagreeing too much with each other.
I think I shall rain on the parade, too: let’s return the thread to its rightful owner, the chap with against his name.
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:we could have returned a constant (perhaps equal() to Integer.MIN_VALUE) that was unambiguous in its meaning.


Yes, it's a sticky problem worthy of scrutiny and discussion. The problem is finding a constant that has unambiguous meaning and would eliminate most(?) of the need for additional checks. A constant that is equal() to Integer.MIN_VALUE doesn't fit the bill though because it's still ambiguous; equals() is, after all, transitive. If (A equals B) and (B equals C) then it follows that (A equals C). So you'd still be stuck because (NEGATIVE_INFINITY equals Integer.MIN_VALUE) and (Integer.MIN_VALUE equals Unambiguous_Constant) then (NEGATIVE_INFINITY equals Unambiguous_Constant) still leads you to ambiguity. The best I can do in this case is to agree on a clear convention and the proposed "Integer.MIN_VALUE is equivalent to NEGATIVE_INFINITY for empty sets" is good enough for me. Again, I can accept it as computational convention but not as a mathematical fact.
 
Winston Gutkowski
Bartender
Posts: 10780
71
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Junilu Lacar wrote:A constant that is equal() to Integer.MIN_VALUE doesn't fit the bill though because it's still ambiguous; equals() is, after all, transitive...


You're missing the point. Sure, it might be equal() to some valid value, but it can't be '==' to it. So, just like you would if it was null, you check for it specifically after calling the method, viz:However, I tend to agree with Campbell, time to return this thread to its owner (although, like Elvis, he's probably left the building).

Winston
 
Junilu Lacar
Sheriff
Posts: 17644
300
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Winston Gutkowski wrote:You're missing the point. Sure, it might be equal() to some valid value, but it can't be '==' to it.


Got it. But you should have said "==" in the first place, not equal() -- excellent suggestion, and I think this time it's irrefutable. I'll keep it in mind if/when I run into a situation like this.

Apologies to the OP but I do think this was a pretty good discussion. Thanks to everyone for your patience!

Now, back to your regular programming...
 
This is my favorite show. And this is my favorite tiny ad:
a bit of art, as a gift, the permaculture playing cards
https://gardener-gift.com
reply
    Bookmark Topic Watch Topic
  • New Topic