Clean Code - boilerplate code

[Radoslaw Zubek]

I have problem with some way of encapsulating boilerplate code. To avoid writing some part of code for http requests and responses each time, I decided to encapsulate it in one method. This method gets only query string and listener for response.

void performRequest(String queryString, ResponseListener listener) {...}
 
...
 
interface ResponseListener {
    void httpResponse(int status) throws BaseException;
}

My problem is, that I have to throw a various exceptions depends on 'status' variable and this exceptions are thrown through 'BaseException'.

 
public void doSomeWork(String params) throw DerivedException1, DerivedException2 {
    try {
        performRequest(params,
            new ResponseListener() {
                public void httpRequest(int status) throws BaseException {
                    switch (status) {
                    case 0:
                        /* ok */
                        break;
                    case 1:
                        throw new DerivedException1();
                    case 2:
                        throw new DerivedException2();
                    default:
                        throw new RuntimeException("status: " + status);
                    }
                }
            });
    }
    catch (BaseException e) {
        if (e instanceof DerivedException1) {
            throw (DerivedException1) e;
        }
        if (e instanceof DerivedException2) {
            throw (DerivedException2) e;
        }
    }
}

My question is how to avoid this ugly code for casting exceptions and encapsulate some boilerplate code.

[Ilja Preuss]

Unfortunately, I don't know of a good solution in Java.

If the number of possible exceptions is limited and small, you could use generics:

interface ResponseListener<E1 extends Exception, E2 extends Exception> {
    void httpResponse(int status) throws E1, E2;
}

This is far from being an elegant solution, though, and becomes unwieldy once the number of exceptions grows or differs between implementations of the interface.

[Paul Croarkin]

My approach to Exception handling is keep throwing until it can be handled. Most low-level exceptions cannot be handled directly by the code.

A good rule is to catch exceptions anytime an actor makes an action.

For instance, in a Web app where the user uploads a file path that does not exist, the proper thing is to tell the user that the file path does not exist. What value does the user get from the program catching some IOException (or worse swallowing it) way down in the code. They want to know what they did wrong or what the system did wrong and how to get around it.

A web service could be an actor and you may be expecting an XML file to come down the wire and it does not parse against the DTD or schema. Log it to an error file (or table) and possibly send an alert (email or page) to the production support team. Also be sure that the response you send back explains the error. Also make sure that you rollback any transactions.

[ September 23, 2008: Message edited by: Paul Croarkin ]
[ September 23, 2008: Message edited by: Paul Croarkin ]

[Ilja Preuss]

Paul, I have no idea how your response addresses the original poster's problem...

[Robert Martin]

Originally posted by Radoslaw Zubek:
I have problem with some way of encapsulating boilerplate code. To avoid writing some part of code for http requests and responses each time, I decided to encapsulate it in one method. This method gets only query string and listener for response.

void performRequest(String queryString, ResponseListener listener) {...}
 
...
 
interface ResponseListener {
    void httpResponse(int status) throws BaseException;
}

My problem is, that I have to throw a various exceptions depends on 'status' variable and this exceptions are thrown through 'BaseException'.

 
public void doSomeWork(String params) throw DerivedException1, DerivedException2 {
    try {
        performRequest(params,
            new ResponseListener() {
                public void httpRequest(int status) throws BaseException {
                    switch (status) {
                    case 0:
                        /* ok */
                        break;
                    case 1:
                        throw new DerivedException1();
                    case 2:
                        throw new DerivedException2();
                    default:
                        throw new RuntimeException("status: " + status);
                    }
                }
            });
    }
    catch (BaseException e) {
        if (e instanceof DerivedException1) {
            throw (DerivedException1) e;
        }
        if (e instanceof DerivedException2) {
            throw (DerivedException2) e;
        }
    }
}

My question is how to avoid this ugly code for casting exceptions and encapsulate some boilerplate code.

Catch the exceptions by their true type!! Don't catch the base and then downcast. Yes, this means you'll have many catch clauses. That's better than having a set of instanceof statements...

[Ilja Preuss]

Originally posted by Robert Martin:

Catch the exceptions by their true type!! Don't catch the base and then downcast. Yes, this means you'll have many catch clauses. That's better than having a set of instanceof statements...

Mhh, yes... but - you will still have to catch BaseException, too, because that is what the interface declares. Of couse you know that it can't be thrown, but unfortunately, the compiler doesn't.

Mhh, just remembered that you prefer RuntimeExcptions. Problem solved.
[ September 25, 2008: Message edited by: Ilja Preuss ]

[Robert Martin]

Originally posted by Ilja Preuss:


Mhh, yes... but - you will still have to catch BaseException, too, because that is what the interface declares. Of couse you know that it can't be thrown, but unfortunately, the compiler doesn't.

Mhh, just remembered that you prefer RuntimeExcptions. Problem solved.

[ September 25, 2008: Message edited by: Ilja Preuss ]

The following code seems to work fine.

  
public class C {
  public static void f() throws E {
    throw new E1();
  }
 
  public static void main(String[] args) {
    try {
      f();
    } catch (E1 e) {
      // do right thing.
    } catch (E2 e) {
      // do right thing.
    } catch (Exception e) {
      // complain bitterly.  
    }
  }
}
 
class E extends Exception {}
class E1 extends E {}
class E2 extends E {}
  

[Ilja Preuss]

Originally posted by Robert Martin:

The following code seems to work fine.

Frankly, I think the third catch block is suboptimal. The only advantage checked exceptions have over non-checked exceptions is that the compiler will tell you if you are not handling them all. With the catch-all block, you loose that advantage - you won't be told automatically when there is a new Exception added that might need to be handled differently. And if you don't get that advantage, you might as well use non-checked exceptions to begin with (which I guess you would do ).