[jira] [Comment Edited] (SOLR-13658) Discuss adding the new "var" construct to the forbidden API list.

["Uwe Schindler (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-13658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16897164#comment-16897164 ] 

Uwe Schindler edited comment on SOLR-13658 at 7/31/19 1:17 PM:
---------------------------------------------------------------

The var keyword should not bring problems, as it does not mean "unchecked". Actually it's even more strict! The compiler introduces the exact type of the right side as the "virtual" type and uses it. So {{var list = new ArrayList<String,String>}} introduces as type {{ArrayList<String,String>}}. Even more funny, if you use an anonymous subclass, the "var" type is the same as the internal type of the anonymous subclass (some Foobar$1$2 class)! This leads to the fact that you can call methods through the "var" type, which are only defined (public) in the anonymous subclass and not in the class it extends!

As explained before, in contrast to "var" in javascript or "def" in Groovy, the type is known to the compiler, so there is no risk! There is maybe only ambiguity if it's not obvious where the type comes from. Famous case: You SHOULDN'T use diamond operator with var!!! I'd add a regex rule for this special case (var in combination with diamond on right side).

You also have to know, as "var" is the real native type; you cant assign something else to a "var" typed variable later on. So if you create an ArrayList and assign it to a "var" typed variable, it has to stay an ArrayList, so assigning a different List type won't work. So it's actually more strict! So when using Interfaces like List and Set, it may often be better to use a real declaration. Java Champignon Heinz Kabutz tells you: Where you should better use interfaces (like List), don't use "var" (for the reasons above, the "var" would be type of concrete class, not interface).

Finally, the var type is not worse than lambdas, they have the same "automatism". it uses the same semantics. If you forbid "var", you also have to forbid "lambdas". :-)


was (Author: thetaphi):
The var keyword should not bring problems, as it does not mean "unchecked". Actually it's even more strict! The compiler introduces the exact type of the right side as the "virtual" type and uses it. So {{var list = new ArrayList<String,String>}} introduces as type {{ArrayList<String,String>}}. Even more funny, if you use an anonymous subclass, the "var" type is the same as the internal type of the anonymous subclass (some Foobar$1$2 class)! This leads to the fact that you can call methods through the "var" type, which are only defined (public) in the anonymous subclass and not in the class it extends!

As explained before, in contrast to "var" in javascript or "def" in Groovy, the type is known to the compiler, so there is no risk! There is maybe only ambiguity if it's not obvious where the type comes from. Famous case: You SHOULDN'T use diamond operator with var!!! I'd add a regex rule for this special case (var in combination with diamond on right side).

You also have to know, as "var" is real the native type; you cant assign something else to a "var" typed variable later on. So if you create an ArrayList and assign it to a "var" typed variable, it has to stay an ArrayList, so assigning a different List type won't work. So it's actually more strict. So when using Interfaces like List and Set, it may often be better to use a real declaration. Java Champignon Heinz Kabutz tells you: Where you should better use interfaces (like List), don't use "var" (for the reasons above, the "var" would be type of concrete class, not interface).

Finally, the var keyword is not worse than lambdas, they have the same "automatism". it uses the same semantics. If you forbid "var", you also have to forbid "lambdas". :-)

> Discuss adding the new "var" construct to the forbidden API list.
> -----------------------------------------------------------------
>
>                 Key: SOLR-13658
>                 URL: https://issues.apache.org/jira/browse/SOLR-13658
>             Project: Solr
>          Issue Type: Wish
>      Security Level: Public(Default Security Level. Issues are Public) 
>    Affects Versions: master (9.0)
>            Reporter: Erick Erickson
>            Assignee: Erick Erickson
>            Priority: Major
>
> Personally, I'm strongly against allowing the "var" construct in Lucene/Solr code. I think it's a wonderful opportunity to introduce bugs that won't be found until runtime as well as making maintainence significantly harder. I don't even think for a project like Solr it would save any time overall...
> So let's discuss this ahead of time and see if we can reach a consensus. I'll start the discussion off:
> My baseline argument is that for a large complex project, especially ones with many different people coding, I want the compiler to give me all the help possible. And the "var" construct takes away some of that help.
> I’ve seen this argument go around at least 4 times in my career. The argument that “it takes longer to write if you have to type all this stuff” is bogus. Last I knew, 80% of the time spent is in maintaining/reading it. So the argument “I can write faster” means I can save some fraction of the 20% of the time writing the original code but spend many times that figuring out what the code is actually doing the other 80% of the time.
> The IDE makes _writing_ this slightly faster, admittedly.
> {code:java}
> Whatever what = new Whatever();
> var kidding = what.getComplex();
> var blivet = kidding.get("stuff");
> {code}
> But once that’s done, if I’m reading the code again I don't have any clue what
> {code:java}
> kidding or blivet
> {code}
> are. Here's the signature for getComplex:
> {code:java}
> Map<String, Map<Integer, Integer>> getComplex()
> {code}
> I have to go over to the definition (which I admit is easier than it used to be in the bad old days, but still) to find out.
> HERE'S THE PART I REALLY OBJECT TO!
> The above I could probably live with, maybe we could get the InteliJ developers and see if they can make hover show the inference. What I will kick and scream about is introducing bugs that are not found until runtime. Even this obvious stupidity fails with a ClassCastException:
> {code:java}
> var corny = new TreeMap<String, String>();
> corny.put("one", "two");
> corny.get(1);
> {code}
> But it's much worse when using classes from somewhere else. For instance, change the underlying class in the first example to return
> {code:java}
> Map<String, Map<String, Integer>>{code}
> . 
>  This code that used to work now throws an error, _but it compiles_.
> {code:java}
> var kidding = what.getComplex();
> var blivet = kidding.get("stuff");
> var blah = kidding.get("stuff").get(1); //  generates ClassCastException: class java.lang.String cannot be cast to class java.lang.Integer
> {code}
> So in order to save some time writing (that I claim will be lost multiple times over when maintaining the code) we'll introduce run-time errors that will take a bunch _more_ time to figure out, and won’t be found during unit tests unless and until we have complete code coverage.
> If there's a way to insure that this kind of thing can't get into the code and we implement that, I could be persuaded, but let's make that an explicit requirement (and find a suitable task for the build system, precommit or whatever).
> The floor is open...



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org