David,
Thank you for reviewing the fix!
On 05/09/2016 03:08 AM, David Holmes wrote:
Hi Kirill,
On 7/05/2016 3:39 AM, Kirill Zhaldybin wrote:
Alexander,
Thank you for reviewing the fix.
On 06.05.2016 20:24, Alexander Kulyakhtin wrote:
Is there a specification of what shall be thrown, or the justification
of the changes done?
One can think that the currently thrown NoSuchMethodException is
correct since　methods, which differ in signature only, are still
different methods.
I spent some time trying to figure out why I got NoSuchMethodException
for "help" despite such method actually existed.
If you think that to throw NoSuchMethodException is better for wrong
signature I could just change error message in this case.
The code doesn't really seem to be checking any detail of the
"signature" as such, simply that the passed in "signature" array has a
non-null zeroeth element of type String[]. Passing something else
seems like a bad call to me and so IllegalArgumentException would be
appropriate I think.
But the logic of that entire block is suspect to me as it does not
handle the case where isEmpty() is true, but we pass in non-null
params and signature anyway.
I tried to make this fix as small and low-risk as possible - as far as
the repo closes on Wednesday
The change does not affect "command execution" - it only throws
different exceptions.
Thank you.
Regards, Kirill
Aside: I'm not even convinced that NoSuchMethodException is
appropriate for the case where "actionname" doesn't exist. But that is
a different issue.
Thanks,
David
Could you please let me know your opinion?
Thank you.
Regards, Kirill
Best regards,
Alexander
----- Original Message -----
From: [email protected]
To: [email protected]
Sent: Friday, May 6, 2016 8:11:08 PM GMT +03:00 Iraq
Subject: RFR(XXS): 8156226: DiagnosticCommandImpl::invoke throws
NoSuchMethodException even if the method actually exists but
parameters are wrong
Dear all,
Could you please review this small fix for 8156226?
A case when a method exists but parameters' signature is wrong now
causes new ReflectionException(new IllegalArgumentException()) thrown
instead of new ReflectionException(new NoSuchMethodException()).
WebRev:
http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8156226/webrev.00/
CR: https://bugs.openjdk.java.net/browse/JDK-8156226
Thank you.
Regards, Kirill
