JDK 12 RFR of JDK-8206085 : Refactor langtools/tools/javac/versions/Versions.java

[Maurizio Cimadamore]

Looks good.

I believe the test could be further automatized, by adding more logic to 
the enum. E.g. each constant could define in a string a snippet of code 
that is unique to that version (and maybe the BASE version would have an 
empty snippet). That way the sources could be generated programatically 
- no need to repeat various headers etc. Also, I believe once you go 
down this path, it could be even possible to have a single checksrc 
method which takes a couple of version constants, generate the sources 
for them using the templating strategy described above.

E.g. it would be great if all the 'mutable' parts would be contained to 
the enum declaration - adding enum constants is rather easy, especially 
when the structure is clearly self-explanatory (as in this case).

Maurizio


On 03/07/18 00:38, joe darcy wrote:
>
> Hi Peter,
>
>
> On 7/2/2018 7:36 AM, Peter Levart wrote:
>> Hi Joe,
>>
>> A suggestion for more compact code: the (ever growing) switch 
>> statement in enum SourceTarget.checksrc could be replaced with a 
>> field of type BiConsumer<Versions, String[]>, initialized in 
>> constructor. For each enum constant you then just pass a method 
>> reference to the chosen target method. The checksrc in SourceTarget 
>> could be an instance method.
>
> I had an intermediate version along those lines before sending the 
> first one out for review. In the intermediate version, I stored in the 
> method reference in a field and the enum client code used an accessor 
> method for it code. The downside was a bit unpleasent to use since the 
> vararg-ness isn't captured in the generic type. But if checksrc was a 
> varags instance on the enum, then it could benefit from the varargs 
> conversion. Let's see ... yes, the resulting test looks better and 
> should be easier to maintain:
>
> http://cr.openjdk.java.net/~darcy/8206085.2/
>
> Thank you for the suggestion.
>
> In any case, the span of versions should reduced soon by the removal 
> of -source/-target/--release 6 in JDK 12 [1], which is one reason I'm 
> working on refactoring these tests now :-)
>
> Cheers,
>
> -Joe
>
> [1] http://mail.openjdk.java.net/pipermail/jdk-dev/2018-May/001210.html
>
>>
>> Peter
>>
>> On 06/29/2018 09:41 PM, joe darcy wrote:
>>> Dis-charged revision along with JDK 11 specific source example now 
>>> up at
>>>
>>> http://cr.openjdk.java.net/~darcy/8206085.1/
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>>
>>> On 6/29/2018 9:55 AM, Jonathan Gibbons wrote:
>>>> There's more static in this file than on my high-school Van de 
>>>> Graaf generator!
>>>>
>>>> Can we follow the convention of creating an instance in main, and 
>>>> then using instance methods.
>>>>
>>>> The naming of some methods is also novel, with respect to 
>>>> case-conventions.
>>>>
>>>> -- Jon
>>>>
>>>>
>>>> On 6/28/18 8:15 PM, joe darcy wrote:
>>>>> Hello,
>>>>>
>>>>> Fresh off of updating
>>>>>
>>>>>     langtools/tools/javac/versions/Versions.java
>>>>>
>>>>> for the JDK 11 -> 12 transition, I'd like to refactor the test to 
>>>>> reduce the maintenance needed when adding new versions (as in new 
>>>>> releases) or removing versions (as is planned for later in JDK 12):
>>>>>
>>>>> http://cr.openjdk.java.net/~darcy/8206085.0/
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Joe
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180703/07a0e99f/attachment-0001.html>