Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Example:

foobar = new InputStreamReader(p.getInputStream(), "ISO-8859-1");

Since the encoding is hardcoded and correct, the constructor will never throw the UnsupportedEncodingException declared in the specification (unless the java implementation is broken, in which case I'm lost anyway). Anyway, Java forces me to deal with that exception anyway.

Currently, it looks like that

try {
    foobar = new InputStreamReader(p.getInputStream(), "ISO-8859-1");
}
catch(UnsupportedEncodingException e) { /* won't ever happen */ }

Any ideas how to make it better?

share|improve this question
1  
For a real challenge, write a unit test to make sure this catch actually works. –  Jay Elston Nov 29 '11 at 16:35
    
`throw new ImpossibleException("Reboot the universe. Things are messed up", e); –  Chris Cudmore Nov 29 '11 at 17:46
    
"Won't ever happen" will... –  user1249 Nov 30 '11 at 10:29
    
Thorbjørn Ravn Andersen: it might after changing the program, but at least in its current state, no set of input data can trigger the exception. –  user281377 Nov 30 '11 at 12:45
    
In your specific example above, an exception is thrown because the method does not know whether it will be called with a supported or not supported encoding. A way around this would be if there was another constructor that assumed a standard charset was given, which wouldn't need to declare that an exception would be thrown. –  jordan May 21 at 22:51

4 Answers 4

up vote 21 down vote accepted

My habit is, just to be on the safe side, to put an assert into the catch block. Someone might change the contents of the try block later, and you do want to know if the code fails don't you?

share|improve this answer
    
Good idea; a simple assert false; doesn't add too much clutter and makes it clear that I assume the catch block will never be entered. –  user281377 Nov 29 '11 at 9:35
4  
@ammoQ, or you can even add a message to make your intent absolutely clear: assert false : "should never happen". –  Péter Török Nov 29 '11 at 9:37
8  
Even better, "Should never happen because Java requires the charset ISO-8859-1 to be supported." –  dan04 Nov 29 '11 at 15:49

If I had been given a cent for every time I've seen an log/error "This should never happen" I would have... well, two cents. But still...

Empty catch blocks makes my spider senses tingle and most good code analyzer tools complain. I would avoid at all costs to leave them empty. Sure, now you know the error can never happen, but a year from now someone does a global search-replace of "ISO-8859-1" and suddenly you may have an extremly hard to find bug.

The assert false suggestion is good, but since they can be disabled at runtime, they are no guarantee. I'd use a RunTimeException instead. Those won't have to be caught by calling classes and if they ever occur you will have a stack trace to give full information.

share|improve this answer
2  
+1 for using a mechanism that cannot be disabled. –  user1249 Nov 30 '11 at 10:31

I've always done it like this:

try {
    foobar = new InputStreamReader(p.getInputStream(), "ISO-8859-1");
} catch(UnsupportedEncodingException e) {
    throw new AssertionError(e);
}

May be a bit verbose (Java is...), but at least you'll get an assertion error when the impossible happens.

If the Java implementation is broken, you'll want to get as good error message as possible, as quickly as possible, instead of just ignoring the impossible. And even if the Java implementation isn't broken, someone could have changed your code to "UTF8" (oops - should it have been "UTF-8"?).

This should have been a runtime exception in the first place. JDK is full of this sort of wrong choices.

share|improve this answer
2  
The real bad decission (or omission if you want) is that there are no pre-defined Charset instances for those 6 charsets that Java requires. foobar = new InputStreamReader(p.getInputStream(), Charset.ISO_8859_1); - now wouldn't that be nicer and avoid any mistake once and forever? –  user281377 Nov 29 '11 at 23:01
2  
The Guava library has tons of these things. Makes life easier. –  user1249 Nov 30 '11 at 10:30
1  
Java 1.7+ does have charset constants defined: docs.oracle.com/javase/7/docs/api/java/nio/charset/…. Use them like so: StandardCharsets.UTF_8.displayName() –  Michael Oct 3 at 16:19

If you are the only developer who will ever get to see this code then I'd say its fine, but if you are not then I would treat it as a real possibility or at least change the "won't ever happen" comment to something more useful.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.