Request for Review: Chain more Exceptions (RuntimeException)

joe.darcy at oracle.com joe.darcy at oracle.com
Thu Aug 18 22:55:04 UTC 2011



On 8/18/2011 1:14 PM, Alan Bateman wrote:
> Sebastian Sickelmann wrote:
>> Hi,
>>
>> i have created a fix for fixing Exception-Chains in case of an 
>> rethrown RuntimeException.
>>
>> I am not quite sure if this is inside the scope of what i 
>> discussed[0][1] with Joe. But it is
>> fixed in the same manner as the patches there.
>>
>> http://oss-patches.24.eu/openjdk8/RuntimeException/REBASED_ON_07ad16388170/ 
>>
>>
>> Someone who wants to review / sponsor this?
>>
> I agree with Andrew that this is good clean-up.
>
> I think the changes to 
> src/share/classes/javax/xml/crypto/NoSuchMechanismException.java need 
> closer examination. Removing the 3 methods that it overloads should be 
> okay. Adding @since 1.8 to the existing constructor is not okay 
> Removing the cause field given that it's serializable, I need to 
> remind myself how this works.
>
> A minor nit but I notice in many places where you've introduced 
> multicatch that you put the vertical bar at the beginning of the next 
> line whereas I think we've mostly been putting it on the right. In 
> src/share/classes/javax/management/relation/RelationService.java I see 
> you've got both. I don't know if coding conventions have been 
> established on this.
>

For multi-catch, I would actually recommend having the multiple 
exceptions listed on a single line, subject go general line length concerns.

Personally I find

    catch(FirstException | SecondException e) {...}

to be more readable than

    catch(FirstException
             | Second Exception) {...}

I agree with Alan that if the sequence of exception types is going to be 
split across lines, it is better to be split as

    catch(First Exception |
             SecondException e)

rather than having the "|" start the subsequent line.

Cheers,

-Joe



More information about the core-libs-dev mailing list