Java Style Guide Tweaks

5 views
Skip to first unread message

Andrew Grieve

unread,
Jan 9, 2023, 11:38:28 AM1/9/23
to java
I'm proposing two changes:
1) Update section talking about desugaring to reflect that we do not support pre-N (N is when default interface methods are natively supported) (review)
2) Encourage broad catch handlers when dealing with IPC (review)

For wordsmithing - please comment on reviews.
For discussion on the changes - please discuss here.

I'll interpret a week of silence as lgtm :)

Tommy Nyquist

unread,
Jan 9, 2023, 11:53:33 AM1/9/23
to Andrew Grieve, java
For 1): That makes sense to me.

For 2): Catching all exceptions is often something that is done by accident, and might catch things it was not intended to catch (and should be caught at a higher level). Instead of catching Exception, I propose we instead catch RuntimeException in these cases.

In the code today we can already catch multiple exceptions, so it would just be one more in the case where you intentionally want to catch more exceptions. All the exceptions mentioned in the current CL (IllegalStateException, IllegalArgumentException, and SecurityException) are RuntimeExceptions, and as such would have been caught. If you want to catch exceptions that happen at runtime like the CL suggests, we should be explicit about that by catching RuntimeException, with the added bonus of being intuitive to understand for new readers.

I also don't think there's any benefit to reading something like:
catch (Exception e) { ... }
over
catch(RemoteException | RuntimeException e) { ... }
rather the latter is easier to understand the intent behind in my view, since as a new reader I see that the author caught the checked exceptions, and I would have an assumption that since they explicitly thought about RuntimeExceptions. If they had only written Exception, my mind would typically think: Are they intentionally catching RuntimeExceptions here?

That being said, regardless when catching broad exceptions, I still think it is nice to add an explanatory comment.

--
You received this message because you are subscribed to the Google Groups "java" group.
To unsubscribe from this group and stop receiving emails from it, send an email to java+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/java/CABiQX1VuxyTT1vYZ82LFvu%2Bt%2B59fojv-6X%3DHJvaqRYrR6pGLrg%40mail.gmail.com.

Andrew Grieve

unread,
Jan 9, 2023, 10:37:09 PM1/9/23
to Tommy Nyquist, Andrew Grieve, java
re: #2 - I thought the benefit might be that it's simpler to catch "Exception" rather than "List of checked exceptions | RuntimeException". I see your point about having the checked exceptions being written out being better for documentation though, so your proposal SGTM.

Andrew Grieve

unread,
Jan 10, 2023, 10:39:56 AM1/10/23
to Andrew Grieve, Tommy Nyquist, java
I've now updated the CL accordingly.
Reply all
Reply to author
Forward
0 new messages