[jira] [Commented] (SOLR-13658) Precommit fail Java "var" until 9x. Fail "var...<>" constructs entirely
[ https://issues.apache.org/jira/browse/SOLR-13658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16906822#comment-16906822 ] Mark Miller commented on SOLR-13658: Weird keyword to single out IMO - non of the new features or keywords will work in a backport, it's happened before and before, what is different here? What I got from Erick was that he didn't like that it's more difficult to determine the actual type visually, but as he mentioned, IDE's make this simple. Personally, I'm still not a big var fan, but not sure why it should be banned anymore than other new stuff we start using in newer versions every time we jump up a Java version. Ban it all baby ;) > Precommit fail Java "var" until 9x. Fail "var...<>" constructs entirely > --- > > 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), 8.2 >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Fix For: 8.3 > > Attachments: SOLR-13658.patch, SOLR-13658.patch > > > 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> 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(); > 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>{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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (SOLR-13658) Precommit fail Java "var" until 9x. Fail "var...<>" constructs entirely
[ https://issues.apache.org/jira/browse/SOLR-13658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16904690#comment-16904690 ] Erick Erickson commented on SOLR-13658: --- Mis-typed the Jira number, moving commit messages over here. Master: Commit f6f1b4244c40e5665b20a2a8ef9852c6dd827cb2 in lucene-solr's branch refs/heads/master from Erick Erickson [ [https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=f6f1b42] ] SOLR-13658: Precommit fail Java var until 9x. Fail var... 8x: Commit 5df5df9ec37c9fbaca9c0629482a95fb90c8d33b in lucene-solr's branch refs/heads/branch_8x from Erick Erickson [ [https://gitbox.apache.org/repos/asf?p=lucene-solr.git;h=5df5df9] ] SOLR-13658: Precommit fail Java var until 9x. Fail var... (cherry picked from commit f6f1b4244c40e5665b20a2a8ef9852c6dd827cb2) > Precommit fail Java "var" until 9x. Fail "var...<>" constructs entirely > --- > > 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), 8.2 >Reporter: Erick Erickson >Assignee: Erick Erickson >Priority: Major > Attachments: SOLR-13658.patch, SOLR-13658.patch > > > 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> 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(); > 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>{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-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org