Allow in tests, forbid elsewhere is my vote and simple to implement.
Kind Regards,
Brandon
On Wed, Oct 30, 2024 at 6:46 AM Štefan Miklošovič
<smikloso...@apache.org> wrote:
>
> against vars:
>
> Benjamin
> Stefan
> Benedict
> Yifan
>
> somewhere in the middle:
>
> Dinesh
>
> Caleb +1 in tests and +0 in prod code.
> David - against flat out banning, limiting it just for some places
>
> Josh prod / test on/off switch.
>
> for:
>
> Jon
>
> Correct me if I am wrong and I have "miscategorized" somebody.
>
> From what I see, I think the outcome is that we might use this in tests and
> let's just forbid it in prod code. My reasoning behind this is that we might
> all learn how to live with this in tests and we may re-evaluate in the future
> if its usage proved to be beneficial and we are comfortable to have it in
> prod code as well (or we go to ban its usage in tests too). In other words,
> if the majority just can't live without this in prod code we can take a look
> at this again in the future.
>
> Not banning it in tests does not mean that from now on we need to use vars
> there. It just means that tests which will introduce the usage of vars will
> not be rejected.
>
> Does this sound reasonable to everybody?
>
> On Wed, Oct 30, 2024 at 12:31 PM Benjamin Lerer <ble...@apache.org> wrote:
>>
>> I recently faced some var usage in the CQL layer part where it was making
>> the code pretty hard to understand. I am in favor of prohibiting it.
>>
>> Le mar. 29 oct. 2024 à 20:32, Caleb Rackliffe <calebrackli...@gmail.com> a
>> écrit :
>>>
>>> Josh's example of "good" usage seems defensible, because the declared type
>>> is already obfuscated to Collection anyway, and my eyeballs are going to
>>> skip to the right anyway to see the instantiated type. I'm +0 on
>>> prohibiting it in non-test code, and -1 on prohibiting it in tests.
>>>
>>> On Tue, Oct 29, 2024 at 2:23 PM Jon Haddad <j...@rustyrazorblade.com> wrote:
>>>>
>>>> When I first saw var as a Java keyword, my initial reaction was one of
>>>> skepticism for the reasons already mentioned. However, in practice, I've
>>>> been using var for years across many code bases (Java and Kotlin) and have
>>>> never had an issue with readability. I find it to be significantly more
>>>> pleasant to work with.
>>>>
>>>> Jon
>>>>
>>>>
>>>>
>>>>
>>>> On Tue, Oct 29, 2024 at 12:13 PM Štefan Miklošovič
>>>> <smikloso...@apache.org> wrote:
>>>>>
>>>>> I get that there might be some situations when its usage does not pose
>>>>> any imminent problem but the question is how we are going to enforce that
>>>>> _at scale_? What is allowed and what not ... Sure we can have a code
>>>>> style for that, but its existence itself does not guarantee that
>>>>> everybody will adhere to that _all the time_. People are people and make
>>>>> mistakes, stuff is overlooked etc.
>>>>>
>>>>> Upon the review we will argue whether "this particular usage of var is
>>>>> meaningful or not" and "if the codestyle counts with this or not" and as
>>>>> many reviews we will have there will be so many outcomes. Dozens of
>>>>> people contribute to the code and more than that reads it, everybody has
>>>>> some opinion about that ... Sure, just use var when you prototype etc but
>>>>> I don't think it is a lot to ask to merge it without vars. Come on ...
>>>>>
>>>>> On Tue, Oct 29, 2024 at 8:08 PM Yifan Cai <yc25c...@gmail.com> wrote:
>>>>>>
>>>>>> I am in favor of disallowing the `var` keyword.
>>>>>>
>>>>>> It does not provide a good readability, especially in the environments
>>>>>> w/o type inference, e.g. text editor or github site.
>>>>>>
>>>>>> It could introduce performance degradation without being noticed.
>>>>>> Consider the following code for example,
>>>>>>
>>>>>> Set<String> allNames()
>>>>>> {
>>>>>> return null;
>>>>>> }
>>>>>>
>>>>>> boolean contains(String name)
>>>>>> {
>>>>>> var names = allNames();
>>>>>> return names.contains(name);
>>>>>> }
>>>>>>
>>>>>> Then, allNames is refactored to return List later. The contains method
>>>>>> then runs slower.
>>>>>>
>>>>>> List<String> allNames()
>>>>>> {
>>>>>> return null;
>>>>>> }
>>>>>>
>>>>>>
>>>>>> - Yifan
>>>>>>
>>>>>> On Tue, Oct 29, 2024 at 11:53 AM Josh McKenzie <jmcken...@apache.org>
>>>>>> wrote:
>>>>>>>
>>>>>>> (sorry for the double-post)
>>>>>>>
>>>>>>> Jeremy Hanna kicked this link to a style guideline re: inference my
>>>>>>> way. Interesting read for those that are curious:
>>>>>>> https://openjdk.org/projects/amber/guides/lvti-style-guide
>>>>>>>
>>>>>>> On Tue, Oct 29, 2024, at 2:47 PM, Josh McKenzie wrote:
>>>>>>>
>>>>>>> To illustrate my position from above:
>>>>>>>
>>>>>>> Good usage:
>>>>>>>
>>>>>>> Collection<String> names = new ArrayList<>();
>>>>>>>
>>>>>>> becomes
>>>>>>>
>>>>>>> var names = new ArrayList<String>();
>>>>>>>
>>>>>>>
>>>>>>> Bad usage:
>>>>>>>
>>>>>>> Collection<String> names = myObject.getNames();
>>>>>>>
>>>>>>> becomes
>>>>>>>
>>>>>>> var names = myObject.getNames();
>>>>>>>
>>>>>>>
>>>>>>> Effectively, anything that's not clearly redundant in assignment
>>>>>>> shouldn't use inference IMO. Thinking more deeply on this as well, I
>>>>>>> think part of what I haven't loved is the effective splitting of type
>>>>>>> information when constructing generics:
>>>>>>>
>>>>>>> Map<InetAddressAndPort, RequestFailureReason> failureReasonsbyEndpoint
>>>>>>> = new ConcurrentHashMap<>();
>>>>>>>
>>>>>>> vs.
>>>>>>>
>>>>>>> var failureReasonsByEndpoint = new
>>>>>>> ConcurrentHashMap<InetAddressAndPort, RequestFailureReason>();
>>>>>>>
>>>>>>>
>>>>>>> I strongly agree that we should optimize for readability, and I think
>>>>>>> using type inference to the extreme of every case where it's allowable
>>>>>>> would be the opposite of that. That said, I do believe there's cases
>>>>>>> where judicious use of type inference make a codebase more readable
>>>>>>> rather than less.
>>>>>>>
>>>>>>> All that said, accommodating nuance is hard when it comes to style
>>>>>>> guidelines. A clean simple policy of "don't use type inference outside
>>>>>>> of testing code" is probably more likely to hold up over time for us
>>>>>>> than having more nuanced guidelines.
>>>>>>>
>>>>>>> On Tue, Oct 29, 2024, at 2:19 PM, Štefan Miklošovič wrote:
>>>>>>>
>>>>>>> Yes, for now it is pretty much just in SAI. I wanted to know if this is
>>>>>>> a thing from now on or where we are at with that ...
>>>>>>>
>>>>>>> I am afraid that if we don't make this "right" then we will end up with
>>>>>>> a codebase with inconsistent usage of that and it will be even worse to
>>>>>>> navigate in it in the long term.
>>>>>>>
>>>>>>> I would either ban its usage or allow it only in strictly enumerated
>>>>>>> situations. However, that is just hard to check upon reviews with 100%
>>>>>>> accuracy and I don't think there is some "checker" to check allowed
>>>>>>> usages for us. That being said and to be on the safe side of things I
>>>>>>> would just ban it completely.
>>>>>>>
>>>>>>> Sometimes I am just reading the code from GitHub and it might be also
>>>>>>> tricky to review PRs. Not absolutely every PR is reviewed in IDE, some
>>>>>>> reviews are given without automatically checking it in IDE too and it
>>>>>>> would just make life harder for reviewers if they had to figure out
>>>>>>> what the types are etc ...
>>>>>>>
>>>>>>> On Tue, Oct 29, 2024 at 7:10 PM Brandon Williams <dri...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> On Tue, Oct 29, 2024 at 12:15 PM Štefan Miklošovič
>>>>>>> <smikloso...@apache.org> wrote:
>>>>>>> > I think this is a new concept here which was introduced recently with
>>>>>>> > support of Java 11 / Java 17 after we dropped 8.
>>>>>>>
>>>>>>> To put a finer point on that, 4.1 has 3 hits, none of which are valid,
>>>>>>> while 5.0 has 172. If 'sai' is added to the 5.0 grep, 85% of them are
>>>>>>> retained.
>>>>>>>
>>>>>>> Kind Regards,
>>>>>>> Brandon
>>>>>>>
>>>>>>>
>>>>>>>
