|
mrcellux wrote: value of immutability
Only of value when writing code specifically to be used by external third party sources. If you are writing within your own application then mainly unnecessary... unless of course you dont trust your own code?
mrcellux wrote: issues with hungarian annotation
Are you serious? I was simply using an existing and well known example to highlight a *possible* use. By all means change it to be whatever notation you want. When one of your main 4 points is to complain that the naming of the method should be the reason why the mechanical behavior of the method itself is wrong, you know you clearly are grasping at straws.
mrcellux wrote: preference functions over procedures with out arguments
Highly arbitrary and very specific to the actual problem space being addressed. Perhaps if you were returning multiple complex objects with a single call, or if you were a heavy reflection user and were too lazy to spend the extra time in getting the syntax correct to call the functions, but there isn't anything which would be of significant value to warrant the creation of an entirely new class structure. I can absolutely guarantee you that performance-wise the instantiation of your class would have a higher overhead in both cycles and memory than the choice to use an "out" parameter.
mrcellux wrote: generics
How does generics change the preference of one over the other? It doesn't matter whether I am returning a generic class or passing a generic parameter, they are both equally the same.
mrcellux wrote: "I suppose it is tempting, if the only tool you have is a hammer, to treat everything as if it were a nail."
How interesting you should say that... because when presented with a nail, your choice of tool is a hydraulically powered, carbon nano-fibre, diamond tipped power hammer with a nuclear generator that will continue to strike the nail for the next 50,000 years.
mrcellux wrote: Compare
And here you prove my point perfectly.
You have purposely created an example with a verbose, unnecessary and overly complicated example by which to indicate that your code is better. Not only did you purposely ignore the structure of the example given (and I am overlooking the fact you wrote syntactically bad code, you may want to add a bracket before the semi-colon there and add an "out"), but you have no problem with the creation of additional code but only in the example you are trying to disprove. There is that strawman nuclear hammer again
What is worse, the example you gave completely contradicts all the reasons you previously stated as being the whole point of using your original solution. You dont have any choice in your example, you purposely didn't instantiate the return variable so you could make your code look smaller, but by doing so you have FORCED your own code to deal with the absence of the entity straight away. What's more, I no longer have access to the result as it is now gone.
So nice once... you purposely set up two examples in an attempt to try and make your solution look good, then make sure the examples are vastly different just to make a point.
Very. Poor. Coding
But to correct your mistakes and give a fairly equivalent example
Holder<User> userHolder = findUser("foo");
userHolder.ifPresent(user ->> mail(user, "bar"));
My example (when removing all of your unnecessary bumpf)
Holder<User> userHolder = null;
if ( tryFindUser("foo", out userHolder) )
{
mail(userHolder, "bar");
}
Now the second you need to do something more complex when the value exists, your example falls down as your use of LINQ becomes unweildly with more than a single method call being done, while my control structure becomes necessary.
So the irony here is that YOUR solution very much looks like you have a hammer, and every solution is solved when you make a single method call when you have a value (ie your nail).
Well done
|
|
|
|
|
Hehe. If you consider your code example superior/simpler then we don't have much to talk about. Let's agree to disagree.
That code spaghetti with mutable values, temporal null assignment, out argument presented as the silver bullet recommended way.. God save us from the coding horror of single, declarative statements, and blasphemous paradigms.
Ps: you forgot to grab the user from the holder when you mail. Also you can have multiple statements in that lambda, there is no such limitation you suggest.
modified 26-Nov-15 13:08pm.
|
|
|
|
|
Who ever said my example was superior? And who ever said it was a silver bullet?
In fact I made it very clear I was simply correcting YOUR example which you purposely made to appear worse in order to try and make your own example better (while failing some of the original design considerations YOU actually considered to be of value such as not having to deal with the result immediately).
I know you can have multiple statements in a lambda, but surely even you know that doing so is a last minute cludge for several reasons, the least of which is the scope in which it runs.
Its funny... I thought I made it clear that you use the right tool for the right job, yet TWICE now you have tried to claim that I am pushing some kind of "one solutions fits all", first with the abhorrent hammer analogy, and now by making up your own story about me thinking a bit of corrected code to bring the two examples more on equal footing (and remove your bias) was a silver bullet.
Now if you were talking about the use of event arguments, your solution is basically EXACTLY what is already in place to cover such a paradigm, because the need to send serialized arguments fits the bill exactly. Similarly if you were dealing with a background worker, which takes a single "object" as both parameters and results, I would say your solution is also the best method for the task at hand... but only because both of these things are 40 foot tall nails where you have no choice BUT to align yourself within the mechanism provided, hence your nuclear powered jack hammer works well.
Problem is, YOU are the one presenting your solution as if it fits everything, which I originally pointed out for simple tasks it was horrendously over engineered. Instead of going "Oh I agree, you wouldn't use this for something that simple" you doubled down and tried to exaggerate your solution as being a better fit for everything.
Projection thy name is mrcellux
|
|
|
|
|
Mate, the functional code with the option monad is much better for the problem than the alternative you propose.
There is an article comparing the same things:
The Option Pattern
Something like bool tryParse(something, output) might be a smashing idea for high-scale streaming of the results. Otherwise it's just cumbersome. Try to code in Haskell a bit, it will shape your C# too in good ways.
|
|
|
|
|
Firstly I wasn't suggesting that my solution was the most optimal, I was pointing out the over-engineering of the suggested solution for something as simple as just obtaining a value.
Secondly, when a pattern has to write something like this:
"The worst is that clients may accidentally refer to value when TryGetValue returns false."
It just shows how much they are grasping at straws. People will always write bad code, and they will always make accidents, claiming that the potential for bad code writing as being the reason why using that code isn't good is just pure ignorant.
"Oh don't use the if statement, you might get your equals and not equals mixed up, so the fact you could 'accidentally' compare with the wrong operator means that the if structure is a bad choice"
Its the same thing.
The OPs tried to point out that hungarian notation of the method name was one of the 4 reasons why I was wrong. Are you serious? Its a method name, call it whatever you want, that doesn't change the behavior of the function, and it doesn't mean the function is bad because you named it a certain way.
Like I said... grasping at straws.
At the end of the day, I need to get something, and I need to know if I succeeded or not. Depending on what it is I got, and how detailed the result is for determining if I got it determines WHICH is the better method to use. If all I am getting is an integer value out of a string, then why wouldn't I just use TryParse? If I am attempting to retrieve a patient and there are several known reasons why I could fail, each of which I may want to code separately for then I might return an enumeration, if I need to return several complex structures with a combined return state that may contain several exceptions or errors, then I would possibly use the provided solution in this post... The point is that you use what is most appropriate based on the needs at the time.
THAT was the point I was making. No silver bullets, no one solution fits all, and of course there are other solutions that work better in other situations.
|
|
|
|
|
Tell me the exact problems you see in my code example? Where is it overengineered, where is the bloat, why is it inelegant, why should the developer of that code quit programming, why is it a joke? Your statements. Concretes please, just the code snippet. I used the silvet bullet term because you apparently get angry when you see an alternative different to your usual stuff.
The problem i see with your example a bit more distilled: neither the Holder nor the bool have semantic meaning of their own, they can only be used together. Since the method can only return a single value, you introduce the out parameter as a workaround for that. Since you need both those to be useful, you won't be able to pass it to a field of an object, even passing it to a method is more troublesome. This is why its difficult to delay handling. You have no such problems if you simply utilize the type system to group these two. Very natural thing to do so, and serves an important purpose.
It enables then comply some good principles like side effect free functions, and immutability. Sure, you can continue to violate those 2 principles (arbitrary as you said) but you lose a lot in the process.
Those principles enable you powerful stuff like:
new List<string>({"stan", "pan"})
.Select(name => findUser(name))
.Where(user => user.isPresent()))
.ForEach(user => mail(user, "bar"));
And this is just scratching the surface. Statistically one needs about half the code with such coding style and is less error prone (the research result i read on this was based on java7 - scala comparison)
|
|
|
|
|
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.
|
|
|
|
|