[jira] [Commented] (SOLR-13658) Precommit fail Java "var" until 9x. Fail "var...<>" constructs entirely

2019-08-13 Thread Mark Miller (JIRA)


[ 
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

2019-08-11 Thread Erick Erickson (JIRA)


[ 
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