10

Today I was working with Java class StringReader and I found it very annoying that it throws IOException on read method. I know that it extends Reader class in which method read throw IOException but I think that it is not needed for StringReader. This class doesn't use any external resources that might cause errors.

After short investigation I found out that StringReader#read throws IOException if string which this class reads is null but de facto this can't happen because if we would try to pass null to StringReader constructor it throws NPE.

What do you think about it, is it good practice to always throw the same exceptions as super class?


Edit: As noted by U Mad Reader is a class not interface.

Paweł Adamski
  • 2,765
  • 2
  • 21
  • 40
  • 1
    The throws declaration in `StringReader` is mandatory, because the interface `Reader` requires it even if the implementing class won't never throws such an exception. – a_horse_with_no_name Apr 17 '14 at 07:27
  • What are the reasons for down votes ? – Damian Leszczyński - Vash Apr 17 '14 at 07:31
  • Reader is a class, not an interface and you can override a function with an implementation with fewer or no exceptions declared. And it will work. So there's no reason for StringReader to have them declared. – RokL Apr 17 '14 at 07:32
  • @UMad, that is true but the Reader is abstract and the method read came from Readable interface. – Damian Leszczyński - Vash Apr 17 '14 at 07:35
  • Doesn't matter. The makers of StringReader could declare read without IOException. That was a mistake on their part. – RokL Apr 17 '14 at 07:41
  • @UMad, Mark Reinhold the developer of StringReader class has done it perfectly. The mistake is on your side as you think that IOExcpetion is StringReader is not valid. – Damian Leszczyński - Vash Apr 17 '14 at 08:05
  • If he declared `read` (and other methods) without the exception then the following would hold true: `Reader r = new StringReader; r.read();` would require exception handling. `StringReader r = new StringReader;r.read();` would not require exception handling. – RokL Apr 17 '14 at 08:10

3 Answers3

7

I think it is not a good practice to throw the same exceptions as the super class or interface definition if your implementation ensures it will never happen. I would always reduce a signature to the bare minimum required.

The IOException is required for all the implementations imaginable, including file sources and streams and sockets etc. Without, such implementations could not notify their errors as a checked exception. But if an implementation has no need to throw a checked exception (which is often annoying for the calling code) removing it from the implementing class does no harm, but removes some burden.

UPDATE:

I have found the reason why the method read() must throw an IOException: because of the contract defined for the close() method. From the JavaDoc:

Closes the stream and releases any system resources associated with it. Once the stream has been closed, further read(), ready(), mark(), or reset() invocations will throw an IOException. Closing a previously closed stream has no effect.

Harmlezz
  • 7,524
  • 23
  • 34
  • haah, the worderful world of checked exception :-) – senseiwu Apr 17 '14 at 07:38
  • You can't do it here. – Braj Apr 17 '14 at 07:52
  • 2
    @Braj: I do not know what's wrong? `ensureOpen()` does not explain why the `IOException` must be thrown. Only if you follow the code further to the `close()` method you find the real cause. And besides: it is a platform for answering questions and not for winning a price. So keep cool :-) – Harmlezz Apr 17 '14 at 07:55
  • Its actually `ensureOpen()` method that throws it. Then I investigated how `this.str` can be assigned to `null` then I come to `close()` method. – Braj Apr 17 '14 at 07:59
5

Please have a look at StringReader#read().

Look at the source code of StringReader#read() method. It calls ensureOpen() method that is actually throwing IOException because ensureOpen() checks to make sure that the stream has not been closed.

If reader is closed and then after read() is called again then what will happen?

Source Code directly from above link (Look at the comments):

/**
 * Reads a single character.
 *
 * @return     The character read, or -1 if the end of the stream has been
 *             reached
 *
 * @exception  IOException  If an I/O error occurs
 */
public int read() throws IOException {
    synchronized (lock) {
        ensureOpen();
        if (next >= length)
            return -1;
        return str.charAt(next++);
    }
}

/** Check to make sure that the stream has not been closed */
private void ensureOpen() throws IOException {
    if (str == null)
        throw new IOException("Stream closed");
}

/**
 * Closes the stream and releases any system resources associated with
 * it. Once the stream has been closed, further read(),
 * ready(), mark(), or reset() invocations will throw an IOException.
 * Closing a previously closed stream has no effect.
 */
public void close() {
    str = null;
}
Braj
  • 44,339
  • 5
  • 51
  • 69
  • 1
    I did it before asking a question :). I found out that ensureOpen checks if input string is not null, which CAN'T HAPPEN. – Paweł Adamski Apr 17 '14 at 07:43
  • But for safer side a expert Java programmer has put this check. – Braj Apr 17 '14 at 07:46
  • If reader is `closed` then after `read` is called again then what will happen? – Braj Apr 17 '14 at 07:47
  • It can happen. I checked the constructor and found a `NullPointerException` would be thrown before. But then I searched the whole code and found the close() method, modifying the str member variabel. Reading the JavaDoc lead to the conclusion, that read() must throw the IOException. – Harmlezz Apr 17 '14 at 07:53
  • @Harmlezz That's what I have already asked in my last comment. – Braj Apr 17 '14 at 07:54
1

When you implement a method of interface in your class it is not required to provide same exception arguments. Same case apply when you override declaration of method from super class.

public class MyReader implements Readable {

    @Override
    public int read(CharBuffer cb)  {
        return 0;
    }

}

But then you are not using the interface in proper way. And you do not benefit from this in case when you are coding to interface.

Readable readable = new MyReader();

        try {
            readable.read(null);
        } catch (IOException e) {
            e.printStackTrace();
        }

Even in the MyReader do not expose the IOException you still have to use try block. So in case you do not throw exception of the method you implement may point that you missed something in your implementation of that method. So IMHO it is not a good practice do to so.

The reason why StringBuilder throws IOException is not that it implement the interface Readable. The reason of that is validation of the input in method ensureOpen() that throw an IOException when input is null. Then input can be null when method close() is called or you pass null to constructor. As method close is abstract it must have some effect in the child class. The expected is that after you call close you can no longer read from it and you will get IOException.

This is perfect, clean and solid implementation that take into account all potential use cases.