I am fine with allow in tests ban in src
> On Oct 30, 2024, at 7:16 AM, Štefan Miklošovič <smikloso...@apache.org> wrote:
>
> https://issues.apache.org/jira/browse/CASSANDRA-20038
>
> On Wed, Oct 30, 2024 at 2:40 PM Brandon Williams <dri...@gmail.com
> <mailto:dri...@gmail.com>> wrote:
>> 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 <mailto: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
>> > <mailto: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
>> >> <mailto: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
>> >>> <mailto: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 <mailto: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
>> >>>>> <mailto: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
>> >>>>>> <mailto: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
>> >>>>>>> <mailto:dri...@gmail.com>> wrote:
>> >>>>>>>
>> >>>>>>> On Tue, Oct 29, 2024 at 12:15 PM Štefan Miklošovič
>> >>>>>>> <smikloso...@apache.org <mailto: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
>> >>>>>>>
>> >>>>>>>
>> >>>>>>>
