RFR: JDK-8223914: specification of j.l.c.MethodTypeDesc::of should document better the exceptions thrown
Vicente Romero
vicente.romero at oracle.com
Mon May 20 19:29:05 UTC 2019
Hi Roger,
On 5/20/19 2:00 PM, Roger Riggs wrote:
> Hi Vicente,
>
> In the CSR, the Summary should be about the change...
> "...MethodTypeDesc::of should document all exceptions.
> Avoid duplication between Summary and Problem.
thanks, I saw that you already modified the Summary
>
> I would omit the part about "content of parameter" or "its contents"
> is null;
> It cannot happen and if it does, its more of an internal error than a
> regular NPE
> since it should not be possible to create a ClassDesc with a null
> descriptor string.
this spec comment was included in this one an other similar methods. It
is probably a bit of an overkill but we have already changed the spec of
several similar methods to this pattern, see for example [1]
>
> In the Code Review:
>
> MethodTypeDesc.java: 68: as above, "or its contents are" -> "is "
> there's no need to mention the contents.
>
> MethodTypeDescTest.java: 264 and 274. The messages would be more
> useful (if they ever were to happen)
> and for the person reading the code if they describe what should not
> happen.
> For example, "ClassDesc array should not be null" or ClassDesc
> should not be null.
I made this change in the test comment [2]
>
> Thanks, Roger
Thanks,
Vicente
[1] https://bugs.openjdk.java.net/browse/JDK-8223803
[2] http://cr.openjdk.java.net/~vromero/8223914/webrev.01/
>
>
> On 05/17/2019 12:55 PM, Vicente Romero wrote:
>> Please review simple fix for [1] at [2] plus the CSR at [3]. This fix
>> is simply documenting all the missing cases in which method
>> java.lang.constant.MethodTypeDesc::of can throw exceptions. A test
>> has been added to cover the missing cases.
>>
>> Thanks,
>> Vicente
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8223914
>> [2] http://cr.openjdk.java.net/~vromero/8223914/webrev.00/
>> [3] https://bugs.openjdk.java.net/browse/JDK-8224136
>
More information about the core-libs-dev
mailing list