review request for 7008728: diamond conversion of basic security, permission, authentication code

[Sean Mullan]

On 12/28/10 2:36 PM, Stuart Marks wrote:
> In the thread for the review of 7008713, you had asked:
>
>> One instance where I think it is debatable as to whether it improves code
>> readability is if the variable is declared somewhere else in the code. In
>> these cases I find myself scrolling upwards or searching to find the
>> declaration to see what type the parameters are.
>
> Yes, this is certainly debatable and worthy of discussion.
>
> Let's look at a couple specific examples. In the webrev for this change, the
> first file (PolicyFile.java) has a couple good examples. At line line 1183
> there's an example of a diamond used in the initializer for a local variable:
>
> ArrayList<Certificate> userCertList = new ArrayList<>();
>
> It's pretty clear the new ArrayList contains instances of Certificate, since the
> declaration is right there. However, earlier in the same file at line 295,
> there's the following change (shown in unified diff format):
>
> - policyEntries = new Vector<PolicyEntry>();
> + policyEntries = new Vector<>();
>
> In this case policyEntries is a field of the enclosing object. Looking at this
> diff, it's not at all clear that this change is correct. In order to determine
> its correctness, you have to hunt around in the file to find the declaration.
>
> But note that policyEntries isn't actually used here so the type parameter isn't
> really significant for this particular bit of code. The type parameter *is*
> significant at other places where policyEntries is actually used (e.g., lines
> 651 and 911), and these are also pretty far from the field's declaration -- and
> these uses are also independent of the diamond operator. So, in order to use the
> field properly, you have to hunt around for the declaration (or ask your IDE to
> find it for you) anyway.
>
> There are probably cases where a field might be declared somewhere, but then
> there's some code that constructs a new instance using the diamond operator and
> stores it in that field, and then uses it immediately. In that case it might be
> helpful to have the type parameters in the constructor since the code right
> below it uses them. Are there any such cases in the code? Probably. I could go
> through more of the code to look for cases like this.
>
> What do you think? Do we want to avoid using diamond in straight assignment
> statements (where the declaration is far away) or is it OK? Would this be a
> general rule, or would it be applied on a case-by-case basis, depending on some
> criterion, e.g. how far away the declaration is?

I don't really have a strong opinion, but I might be annoyed if there were a 
complex set of guidelines as to when to use the diamond operator and when not 
to. Anyway, maybe we should see if the JSR 334 expert group has any additional 
thoughts?

Thanks,
Sean