> On 27 Jun 2016, at 16:16, Claes Redestad <claes.redes...@oracle.com> wrote:
>>
>>>>
>>>> -
>>>> 957 if (!VersionProps.VERSION_PRE.isEmpty()) {
>>>> 958 pre = Optional.of(VersionProps.VERSION_PRE);
>>>> 959 } else {
>>>> 960 pre = Optional.empty();
>>>> 961 }
>>>>
>>>> Encapsulate that and the other two into a separate method e.g.
>>>> optionalOfEmpty then do:
>>>>
>>>> version = new Version(…
>>>> optionalOfEmpty(VersionProps.VERSION_PRE),
>>>> …
>>>> );
>>>
>>> Yes, it'd have been neater if not for the fact that VERSION_BUILD is to be
>>> parsed as an Integer, which complicates it a bit. Maybe it is still an
>>> improvement.
>>>
>>
>> Drat, i missed the Integer.valueOf. I suppose one could change the types in
>> VersionProps to be Optional<String> values, then the semantics would be a
>> little clearer.
>
> Mandy commented off-list yesterday about moving the conversion from String to
> Optional<String> into VersionProps, which I think meshes well with your
> suggestion:
>
> http://cr.openjdk.java.net/~redestad/8160000/webrev.4/
>
> (I still don't think calculating the number of dots is worth our while,
> though)
>
> Does this please?
>
+1
Paul.
