Code Review Request JDK-8215443: The use of TransportContext.fatal() leads to bad coding style

[Xue-Lei Fan]

On 12/17/2018 1:17 PM, Anthony Scarpino wrote:
> It looks like in TransportContext.java:68, you had a mistype that added 
> "fa" to the end of a comment.
> 
Oops, I will update it.

> Also in fatal():267, did you plan to return the exception and have the 
> calling method throw the exception?  As is, the exception is never 
> return and fatal() continues to throw the exceptions.
> 
I considered to return the exception.  I'm not very confident with if I 
searched out all use of the fatal() methods.  For safe, I kept to use 
thrown exception instead. If the method is updated later that there is a 
case that now exception get thrown, there will be a compiling issue. 
Are you OK with it?

Thanks,
Xuelei

> Tony
> 
> On 12/15/18 7:51 AM, Xue-Lei Fan wrote:
>> Hi,
>>
>> Could I have the update reviewed?
>>     http://cr.openjdk.java.net/~xuelei/8215443/webrev.00/
>>
>> The TransportContext.fatal() methods always throw exception. While the 
>> compiler does not aware of it, and may not happy without following a 
>> return statement.  Currently, a lot never executable return statements 
>> are inserted.  As make the code hard to read (thanks for Jamil and 
>> Tony's points).  For example:
>>
>>      shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ...);
>>      return null;    // fatal() always throws, make the compiler happy.
>>
>> In this update, I changed the fatal() method with a return value:
>>
>> -    void fatal(Alert alert, ...
>> +    SSLException fatal(Alert alert, ...
>>
>> Then we can change the use of method as:
>>
>> -    shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ...);
>> -    return null;    // fatal() always throws, make the compiler happy.
>> +    throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ...);
>>
>> The changeset is mostly about removing the never executed return 
>> statements and add the 'throw' keyword to lines that use the fatal() 
>> methods.
>>
>> Thanks,
>> Xuelei
>