> Good usage: >> Collection<String> names = new ArrayList<>(); > becomes >> var names = new ArrayList<String>(); >
This “good” usage quickly becomes bad, as you’re going to have to mix “good” and “bad” together, e.g.: >> var names1 = new ArrayList<String>(); >> Collection<String> names2 = myObject.getNames(); Now, when I want to know type information my eyes have to jump all over the place. This is much worse in longer methods. Reading code is about heuristics for where information is, and your brain executing those swiftly for you, unconsciously. Making it harder to save a few characters of typing - that the IDE already saves you! - is just madness to me. The idea that this helps with complex types or things like Pair is also IMO solving the wrong problem, as we already have style guidance encouraging you to use dedicated types when you need to communicate a concept succinctly. This is always clearer. > On 29 Oct 2024, at 18:47, Josh McKenzie <jmcken...@apache.org> 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
