|
1. "Null check is not performed (hasn't been considered or forgotten)"
Which you then immediately follow with "if(user.isPresent())", which by your own admission could not be performed because it wasn't considered or forgotten.
2. "Time is invested in making sure that implementation never returns a null reference.. Then hope for the best that won't change in the future"
Given that you are the creator of the method, and returning a null is valid, why would you have to invest time in making sure it never returns null? Why would you have to "hope for the best" it doesn't change in the future? How is that any different from someone "hoping for the best" that you dont completely change your structure for Optional<> in the future or change the method calls on it?
3. "Null check is done speculatively, even if it's not needed"
But its ok to use LinQ to perform .Select and .Where "even if its not needed"?
So from your original 3 reasons as to why your solution is better than existing solutions, you have either done the very thing you are saying is wrong with existing solutions, or you have simply replaced one "the developer may write bad code" with a different "the developer may write bad code".
I dont know how many times I have to keep saying this. ALL I did with YOUR example was to try and remove YOUR bias, by removing the superfluous code that you added on purpose to try and make your example look better.
mrcellux wrote: This is why its difficult to delay handling
He says while providing an example that FORCES you to handle it immediately, with absolutely no provision to delay the handling.
Well done!
The only reason I "get angry" as you put it is the sheer hypocrisy and contradiction where you seem to set up these fictitious reasons why something is bad and then violate them yourself in the very next sentence. If "delaying handling" is something you decide is valuable, then why are all your examples completely ignoring the ability to do this? Why are you making a point of showing verbose examples as being bad when the only way to delay handling is to do the very thing you say is bad?
And as I also keep saying, your "powerful stuff" is completely and utterly bloated overkill if all I want to do is parse a simple value. You keep harping on about immutability and claiming I am "violating" it, yet I explained to you that immutability serves a purpose for a reason. If I were to use Entity framework I CANNOT make my data models immutable, it requires them to be changeable in order to load the classes. Does that automatically mean it is "violating the principle" and is therefore bad?
YOU have decided that immutability is a show stopper in ALL circumstances, but look at your very own example. The only think "immutable" about it is that I cannot change the "isPresent" from being true to false (which nobody would ever want to do) and I cannot change the "user" that it relates to (which again I nobody would ever want to do). So immutability is a completely moot point in your example. The only way in which a lack of immutability would be a problem here is if people are purposely writing malicious code.
Are you always so paranoid about your own developers?
"Statistically", you only need about half the code when you write exactly what you want in exactly the way you want for exactly the purpose you want it. So by all means, write an article stating that here is a solution for one very specific problem, and I would totally agree with you (as I said before, event args and background worker parameters/responses is a good example)... but claim that this is something that should be used "anywhere that can return a null" and I will have to disagree because you violate your own premises with very poor convoluted and arbitrary "problems" you manufacture yourself and then solve with your own solution.
|
|
|
|
|
I asked you to pick apart that one line of code, and show me why is it "bad coding".
You jump now to the article and the new example because you can't do so? About the claims:
1) without ifpresent/get you get compile error. Important difference.
2) you being the author is not given. We don't code alone in a cave, we have to communicate through are apis (even with implementations)
3) the linq code is not needed sure. It just makes things safer, shorter, more declarative. You can do this in assembly if you don't consider that important. I think it's our goal as developers to do the most with the least effort.
I didn't say this should be used everywhere. I show a problem, how it solves it and also show the drawbacks (conclusion section). A pattern is chosen or not based on advantage / drawback ratio.
But all that aside, can you do me a favor of talking about a very single thing?
Please return to the original one line of code:
findUser("foo").ifPresent(user -> mail(user, "bar");
You said things like "you have no problem with the creation of additional code", "Very. Poor. Coding". Support your claim of bad code. Shoot. Or is it not bad?
|
|
|
|
|
mrcellux wrote: You jump now to the article and the new example because you can't do so?
Who was the one making the point that being able to delay handling was necessary? So I pointed you to your article which stated this. So "picking apart that one line of code" included the fact that based on YOUR words of what was important, you are not allowing for delayed processing.
That was my issue with your purposefully bias representation of the examples. You went out of your way to make the example supporting your own view as succinct, and then purposely wrote the other example verbosely and grossly bloated just to make a point.
Either delayed processing IS important, or it is not, and if you are going to give examples to illustrate that, then BOTH examples should illustrate that.
I also believe that I did point out how your example is limited in your use of linq. Clearly you have never debugged through a ForEach statement before, or you would understand this, and clearly you have never bothered looking at an exception that is thrown by code inside a ForEach loop either. Your example forces the processing of all items without any capability for handling individual failures.
mrcellux wrote: 1) without ifpresent/get you get compile error. Important difference.
Are you being purposely dense? seriously... Do you honestly think I am talking about removing code which makes it not compile and putting that forward as a valid option? Seriously mate, you come up with some completely insane rebuttals some times. First you argued that Hungarian notation is one of the main reasons that the mechanical behavior of a solution was wrong, NOW you are using inability to compile by making the stupid presumption that what I meant was to just remove code and have it fail.
Is that really your #1 response? A compile error. That is your hands down show stopper of a response?
mrcellux wrote: 2) you being the author is not given. We don't code alone in a cave, we have to communicate through are apis (even with implementations)
And yet another completely useless comment. What on earth does this mean? If you are trying to say that our code needs to explain what it does, I dont see how I wasn't explaining that. Heck, your whole original argument was that null wasn't explicitly indicated in the syntax of the method, which is why I gave you an existing example that does, and I could come up with dozens of others which still don't need your bloated and overweight poor "generic" solution.
mrcellux wrote: 3) the linq code is not needed sure. It just makes things safer, shorter, more declarative
No, you used it specifically to try and make your example look better, which is why you went out of your way to add extra lines of code in the alternate example. Do you really have to go that far out of your way to try and make your solution look good? Shouldn't it just stand on its own two feet?
Seriously, all you needed to do in the beginning was say "sure, if all you need is just a single value then my solution is complete overkill. I stand corrected, my solution isn't for all situations, it is more geared towards those places where you dont have a choice and where you have a need to return more complex data", and I would have been happy. But as I said before, you just doubled down and acted like your solution does everything.
I even handed you a fricken bone and told you that things like event args or background worker request/response classes your solution would be perfect, because in those cases you are FORCED to use only a single object to convey all the information you require... but oh no... dont ever take a step back and correct yourself. Instead, just soldier on, push forward and quote stupid things like "it wont compile if I remove code"
mrcellux wrote: I didn't say this should be used everywhere
Really? I re-read the article looking for ANYWHERE you actually mentioned where this solution should be used and I couldn't find any. What i did find however were lots of "commonly used", "many languages", "many contexts", "an alternative", "with consistent use".... So I am sorry, but failing to explicitly say something doesn't mean you didn't more than imply it with how common and generic you are claiming it is. Even when given the opportunity to clarify it when I pointed it out, instead of going "oh you are right, it shouldn't be used in most cases" you just doubled down again. So even now that you are making a point of NOT saying it should be used everywhere.... you still dont say that it shouldn't be used everywhere.
mrcellux wrote: and also show the drawbacks (conclusion section).
It is interesting you should use the word "drawback". It is only mentioned ONCE in the entire article, and that is to describe the problem you are solving.
But here is your disadvantages:
"Disadvantages? In most cases there aren't, but as to everything there are exceptions. The Optional is a heap object, this needs to be considered in extreme cases. There may be specific scenarios where such practice doesn't deliver real value or is impractical"
Wow... "in MOST cases"... but that sounds like the vast majority of cases SHOULD use this. "Extreme Cases", so just wanting an integer is an extreme case? Wanting a simple object is an extreme case?
Honestly, if you are going to write articles, you need to NOT pat yourself on the back so much and slant everything towards your own solution being some holy grail.
mrcellux wrote: Shoot. Or is it not bad?
I have already been over this a dozen times, and I have even explained it further above, which I have done so many times previously. But what the heck, let me go even deeper into pulling this to pieces.
ifPresent
What an awesome little piece of code. You DO know what the code inside this method is don't you? You do understand the internal compiler directive which performs the is null check and then conditionally executes the next line of code when it exists. So you DO actually realize that anyone can write their own extension function that takes in an expression and performs EXACTLY the same as this. So sorry, you dont get to take credit for other people's coding.
LinQ
You are aware of the overhead of using Linq aren't you? The memory requirements of setting up a functional expression? You are setting up an additional overhead of having to create additional classes for no other reason than to support the Linq interface to make one line of code look pretty. You would achieve this with far less memory and far fewer cycles by simply performing your own null check and then only execute if it exists. Sure it is a few more lines, but doing something PURELY for the code to look good is as I say VERY.POOR.CODING. When it is more important for you to make that line pretty than to think about how the code itself will run and how it will function is bad.
Exception Handling
Said it above, but will repeat it here in case you missed it like all the other times I have spelt out problems. There is no means of handling errors from within Linq statements, you ave not given any indication of where the problem is within the stack trace, and you cannot manage the exception. I noticed that you purposely reduced your example now to leave out the ForEach, which would have caused an even greater problem with handling exceptions. That is VERY.POOR.CODING.
But here is your chance... despite all of this, and you doubling down. Here is your opportunity to explicitly state when this should and shouldn't be used, perhaps even changing the article to reflect this instead of making it sound like there are very few "extreme" cases where it shouldn't be used.
Or are you so protective of your little piece of code that you care more about how it looks than it being correct?
|
|
|
|
|
Cool, this article from 2007 is about the exact same thing as this strangely pointless comment thread here.
|
|
|
|
|
Yes, this is old stuff. You're wasting your time with your comments. That enigma bloke is not interested in discussing consequences of patterns, just wants to be right. You can see that from mile away when someone takes the piss on fundamental design principles in the effort.
|
|
|
|
|
If I just want to be right, then why would I be giving him examples of where his solution would be of use? Why would I be pointing out his lack of scoping it correctly?
I have said it before, if he said this was to only be used in certain instances, rather than make it sound like it is a magic bullet that solves everything and that only 'extreme cases' are outside of it (which is just lip service), then I would not have an issue.
If he didn't purposely make bias examples that are not even functionally equivalent to make a point, and then break his own stated rules of "delayed processing" then I wouldn't have a problem.
I am not taking the piss on fundamental design principles, I am taking the piss out of someone who quotes them poorly, then solves the very problem he creates in the first place.
Show me one design principle I am pissing on?
|
|
|
|
|
In the dictionary class, it seems like the ContainsKey function seems redundant, because when it is true you have to re-execute the logn process to retrieve the location of the value. It seems that it would be more efficient to try to retrieve the value and when the key doesn't exist return null, because both the "if exists" and "retrieve the value" would use one logn process.
Instead you must do two logn processes when one would be more efficient because if you try to retrieve the value without verifying the key exists, the code throws an error. I had great misgivings about trying to retrieve the value in a try block and use a catch block to add the key if it doesn't exist. I was right, that process was extremely slower than using the function to verify the key exists first and creating it when it doesn't.
|
|
|
|
|
Map's containsKey is often needed because get is ambiguous: means either the key is not in the map or it's in the map with a null value.
"Recent" implementations of Map (eg ConcurrentHashMap) don't allow null values. Get is no longer ambigous anymore with such implementations. ContainsKey still has uses if fetching a value has extra cost - eg lazy loading.
I agree, the lookup returning optional would be wrong on such a low level api as a Map. The wrapping costs.. I apply it when that overhead is insignificant and when it provides clarity / safety on my interfaces or local variables. In the majority of cases in practice, but not always.
|
|
|
|
|
something went wrong when editing it. Now it is not a blog anymore, it is an article.
Can you try to revert the change?
M.D.V.
If something has a solution... Why do we have to worry about?. If it has no solution... For what reason do we have to worry about?
Help me to understand what I'm saying, and I'll explain it better to you
Rating helpful answers is nice, but saying thanks can be even nicer.
|
|
|
|
|
I agree with dougbass68 and Alexandru Lungu. There is no difference between checking for null and checking for optional, other than the optional adding another layer of complexity to the compiler. In fact, the simplification for beginners may well do them a disservice: the NULL value is actually a very important concept that is getting swept under the rug with the optional concept.
|
|
|
|
|
Look at the interface side. Thats the most important point actually, don't get hooked up on the checking side. The most important thing is to communicate what to check.
A a()
B b()
C c()
D d()
vs
A a()
B b()
Optional<C> c()
D d()
Which one do you think is the easier / safer / more obvious to use? You also ignored all the other syntax examples.
This thing wasn't included in the jdk for no reason.
modified 11-Nov-15 16:58pm.
|
|
|
|
|
I often see that the benefits of such stuff is not so obvious for beginners. Elaborating on the pain first was a good idea.
|
|
|
|
|
As dougbass68 already suggested, there is no benefit from using user.isPresent() instead of user!=null.
The solution is the "Null-Conditional Operator" "?." (or more known as Safe Navigation Operator) implemented in some languages (C#, Groovy...) and the code now goes from this:
Optional<User> user = findUser("johndoe");
if(user.isPresent())
{
user.get().foo();
}
to
User user = findUser("johndoe");
user?.get().foo();
|
|
|
|
|
I disagree, it does have an important benefit: It makes the nullability explicit on your interfaces / variables and forces you to deal with it. The null-conditional operator can be quite useful for simplifying your navigating nulllable structures but it doesn't provide this kind of visibility at all on your interfaces or variables. It solves something else.
An ideal solution would be disallow null value for all references and return values as default, and allowing them explicitly when needed. Eg
Foo foo
Foo? fooThatIsNullable
The optional tries to mimic something like that on "legacy" languages, but it's a lot less proof / relies on practices.
modified 10-Nov-15 3:51am.
|
|
|
|
|
I don't think that "it makes the nullability explicit" (maybe to very bad programmers); any programmer knows that a reference to an object implies the possibility of being null.
I also don't think it "forces you to deal with it".
People may not check for null (user!=null) the same way people may not check for presence (user.isPresent())
It does make sense in theory:
1) use Optional for objects you must check for existence
2) the lack of Optional means that that object is always not null and should never be checked for nullability.
But in practice, the second point is extremely dangerous - and if you use defensive programming you will check for null anyway, which defeats the purpose of the theory...
|
|
|
|
|
Why would 2) be dangerous? You can make it a convention in your project. It's a lot less risky and more expressive than implicit nulls references, and it does work well in practice. Lots of projects are built like this.
Do I understand correctly that the improvement you suggest is implicit nulls and checking anyway on all references? Do you think the issues don't exist or that they are not solvable?
modified 10-Nov-15 17:17pm.
|
|
|
|
|
Of course you can make it a convention in your project. The problem is that your project is not a completely independent system; it is bound to other systems by the people and other libraries that didn't followed this kind of convention - that is why 2) is dangerous.
I understand the issue, I understand the solution, but implementing it in real life nowadays is too risky.
This solution would have worked perfectly if it was advertised and implemented in the first programming languages and become the common thing.
|
|
|
|
|
Well, we usually know whether we're calling our own api or an external one. I see more and more projects doing this successfully, not theoretical stuff.
We can also expect changes in 3rd party code too in this area. Eg Guava's (almost) no null policy, the java8 stream api. Some related progress began a lot earlier, eg all new Collection implementations that came in jdk5+ prohibit null values.
|
|
|
|
|
Other than the Optional wrapper class adding clarity, I don't see how this avoids littering code with conditionals after method calls. How is calling
if(user.isPresent()) any different than
if(user != null) Also, it looks like it will require and extra step to "get" the underlying type from the returned Optional object.
|
|
|
|
|
I added that code example on purpose to emphasize a similarity between the two. A beginner grasps the concept a lot easier this way.
Towards the end of the article you can find other examples that offer simpler syntax. Eg if you don't need an else you can do:
user.ifPresent(user -> { user.foo(); })
Also note that none of those higher level constructs makes you call that get(). It's only for low level use, and it actually helps by forcing you to think about the value being optional.
In any case, the main point is not the syntax, but the concept of avoiding the use of null references.
|
|
|
|
|
It does seem like a good convention when needed, but I took your "null avoidance paranoia" comment to mean "verbosity", which Optional doesn't seem to reduce.
Can you give me an example of how this would work with a method that returned a collection of User objects? Would the callee and caller need to do anything special to ensure every element is wrapped by Optional?
|
|
|
|
|
Dealing with nulls is not necessarily more verbose, but very likely. For instance, if you have 100 methods in your application, and only 10 may return with "no value", with Optional you will do your checks exactly for those 10 methods. If you refactor and something about this changes, the compiler forces you to adjust you code accordingly. If you just return implicit nulls, you may not know for sure where to do those checks - you may miss some and you may do unnecessary ones. In my experience both mistakes are common - especially as the system keeps changing - and it's simply better not to leave any chance for them to happen.
As for collections, it depends on the context. I can think of scenarios where that may make sense, but most often there is no need for returning Optional elements. If you have 3 elements present, you return a collection of 3 elements unwrapped.
Wrapping in Optional is otherwise easy and can be done at any point where it's needed, eg Optional<user> user = Optional.of(user) (user is surely not null), Optional.ofNullable(user) (user may be null), Optional.empty() (you want to return an empty value).
|
|
|
|
|
In .Net 6, about the only addition I think is of any use is adding the null test operator, but I will probably rarely or never use it. Why? Because I have been burned before. I don't test for nulls, because I don't allow them to exist in my code. My experience (and in my long career I have tried to make every mistake a developer can make) shows that it is a good method.
... The article is just more ambiguous about it than I would be.
|
|
|
|
|
I'm not fully sure what you mean by ambiguity.. The article is exactly about avoiding nulls, and the related issues.
If you mean it could have been more direct, it's true. I could have use a single sentence to explain this to an expert.. but it would be a lot less useful for everyone else. Not just beginners, even a lot of experienced developers tend not to consider practice of nulls a problem, it needs some more explanation / visibility imo.
|
|
|
|
|
Good idea! In TSQL I use the isnull(x,0) <> 0 to ignore 0 value or a null return. Null can cause problems in TSQL when doing comparisons.
Thanks for sharing.
|
|
|
|
|