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
>>>>>
>>>>>
>>>>>
>>>>>
