Yeah, I'm also in favor of disallowing usage of var in production code.
On 2024/10/30 16:43:06 Jacek Lewandowski wrote: > +1 for allow in tests and ban in production code. > > > > > - - -- --- ----- -------- ------------- > Jacek Lewandowski > > > śr., 30 paź 2024 o 17:37 David Capwell <dcapw...@apple.com> napisał(a): > > > 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> 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> 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 > >> >>>>>>> > >> >>>>>>> > >> >>>>>>> > >> > > > > >
