12

Is it a good practice to implement useless exception handling, just in case another part of the code is not coded correctly?

Basic example

A simple one, so I don't loose everybody :).

Let's say I'm writing an app that will display a person's information (name, address, etc.), the data being extracted from a database. Let's say I'm the one coding the UI part, and someone else is writing the DB query code.

Now imagine that the specifications of your app say that if the person's information is incomplete (let's say, the name is missing in the database), the person coding the query should handle this by returning "NA" for the missing field.

What if the query is poorly coded and doesn't handle this case? What if the guy who wrote the query handles you an incomplete result, and when you try to display the informations, everything crashes, because your code isn't prepared to display empty stuff?

This example is very basic. I believe most of you will say "it's not your problem, you're not responsible for this crash". But, it's still your part of the code which is crashing.

Another example

Let's say now I'm the one writing the query. The specifications don't say the same as above, but that the guy writing the "insert" query should make sure all the fields are complete when adding a person to the database to avoid inserting incomplete information. Should I protect my "select" query to make sure I give the UI guy complete informations?

The questions

What if the specifications don't explicitly say "this guy is the one in charge of handling this situation"? What if a third person implements another query (similar to the first one, but on another DB) and uses your UI code to display it, but doesn't handle this case in his code?

Should I do what's necessary to prevent a possible crash, even if I'm not the one supposed to handle the bad case?

I'm not looking for an answer like "(s)he's the one responsible for the crash", as I'm not solving a conflict here, I'd like to know, should I protect my code against situations it's not my responsibility to handle? Here, a simple "if empty do something" would suffice.

In general, this question tackles redundant exception handling. I'm asking it because when I work alone on a project, I may code 2-3 times a similar exception handling in successive functions, "just in case" I did something wrong and let a bad case come through.

5
  • 4
    You are talking about "tests", but as far as I understand your problem you mean "tests which are applied in production", this is better called "validation" or "exception handling".
    – Doc Brown
    May 24, 2013 at 9:53
  • 1
    Yes, the appropriate word is "exception handling".
    – rdurand
    May 24, 2013 at 9:55
  • changed the wrong tag then
    – Doc Brown
    May 24, 2013 at 9:57
  • I refer you to The DailyWTF - are you sure you want to do this kind of testing?
    – gbjbaanb
    May 24, 2013 at 12:02
  • @gbjbaanb: If I understand your link correctly, that's not at all what I'm talking about. I'm not talking about "stupid tests", I'm talking about duplicating exception handling.
    – rdurand
    May 24, 2013 at 14:28

5 Answers 5

14

What you're talking about here is trust boundaries. Do you trust the boundary between your application and the database? Does the database trust that the data from the application is always pre-validated?

That's a decision that has to be made in every application and there are no right and wrong answers. I tend to err on the side of calling too many boundaries a trust boundary, other developers will happily trust third-party APIs to do what you expect them to do, all the time, every time.

5

The robustness principle "Be conservative in what you send, be liberal in what you accept" is what you are after. It is a good principle - EDIT: as long as its application does not hide any serious errors - but I agree with @pdr that it always depends on the situation if you should apply it or not.

5
  • Some people think that the "robustness principle" is crap. The article gives an example.
    – user39685
    May 24, 2013 at 12:31
  • @MattFenwick: thanks for pointing that out, its a valid point, I have changed my answer a little bit.
    – Doc Brown
    May 24, 2013 at 12:56
  • 2
    This is an even better article pointing out the problems with the "robustness principle": joelonsoftware.com/items/2008/03/17.html
    – hakoja
    May 24, 2013 at 13:01
  • 1
    @hakoja: honestly, I know this article well, it is about problems you get when you start not to follow the robustness principle (like some MS guys tried with newer IE versions). Nevertheless, this gets a little bit far away from the original question.
    – Doc Brown
    May 24, 2013 at 14:29
  • 1
    @DocBrown: which is exactly why you should never have been liberal in what you accept. Robustness doesn't mean that you need to accept everything thrown at you without complaint, just that you need to accept everything thrown at you without crashing. May 24, 2013 at 19:25
1

It depends on what it is you're testing; but let's assume the scope of your test is your own code only. In that case, you should test:

  • The "happy case": feed your application valid input and make sure it produces correct output.
  • The failure cases: feed your application invalid inputs and make sure it handles them correctly.

In order to do this, you cannot use your colleague's component: instead, use mocking, that is, replace the rest of the application with "fake" modules that you can control from the testing framework. How exactly you do this depends on the way the modules interface; it can be enough to just call your module's methods with hard-coded arguments, and it can become as complex as writing a whole framework that connects the other modules' public interfaces with the testing environment.

That's just the unit test case, though. You also want integration tests, where you test all the modules in concert. Again, you want to test both the happy case and the failures.

In your "Basic Example" case, to unit-test your code, write a mock class that simulates the database layer. Your mock class doesn't really go to the database though: you just pre-load it with expected inputs and fixed outputs. In pseudocode:

function test_ValidUser() {
    // set up mocking and fixtures
    userid = 23;
    db = new MockDB();
    db.fixedResult = { firstName: "John", lastName: "Doe" };
    db.expectedCall = { method: 'getUser', params: { userid: userid } };
    userController = new UserController(db);
    expectedResult = "John Doe";

    // run the actual test
    actualResult = userController.displayUserAsString(userid);

    // check assertions
    assertEquals(expectedResult, actualResult);
    db.assertExpectedCall();
}

And here's how you'd test for missing fields that are reported correctly:

function test_IncompleteUser() {
    // set up mocking and fixtures
    userid = 57;
    db = new MockDB();
    db.fixedResult = { firstName: "John", lastName: "NA" };
    db.expectedCall = { method: 'getUser', params: { userid: userid } };
    userController = new UserController(db);

    // let's say the user controller is specified to leave "NA" fields 
    // blank
    expectedResult = "John";

    // run the actual test
    actualResult = userController.displayUserAsString(userid);

    // check assertions
    assertEquals(expectedResult, actualResult);
    db.assertExpectedCall();
}

Now things become interesting. What if the real DB class misbehaves? For example, it could throw an exception for unclear reasons. We don't know if it does, but we want our own code to handle it gracefully. No problem, we just need to make our MockDB throw an exception, e.g. by adding a method like this:

class MockDB {
    // ... snip
    function getUser(userid) {
        if (this.fixedException) {
            throw this.fixedException;
        }
        else {
            return this.fixedResult;
        }
    }
}

And then our test case looks like this:

function test_MisbehavingUser() {
    // set up mocking and fixtures
    userid = 57;
    db = new MockDB();
    db.fixedException = new SQLException("You have an error in your SQL syntax");
    db.expectedCall = { method: 'getUser', params: { userid: userid } };
    userController = new UserController(db);

    // run the actual test
    try {
        userController.displayUserAsString(userid);
    }
    catch (DatabaseException ex) {
        // This is good: our userController has caught the raw exception
        // from the database layer and wrapped it in a DatabaseException.
        return TEST_PASSED;
    }
    catch (Exception ex) {
        // This is not good: we have an exception, but it's the wrong kind.
        testLog.log("Found the wrong exception: " + ex);
        return TEST_FAILED;
    }
    // This is bad, too: either our mocking class didn't throw even when it
    // should have, or our userController swallowed the exception and
    // discarded it
    testLog.log("Expected an exception to be thrown, but nothing happened.");
    return TEST_FAILED;
}

These are your unit tests. For the integration test, you don't use the MockDB class; instead, you chain both the actual classes together. You still need fixtures; for example, you should initialize the test database to a known state before running the test.

Now, as far as responsibilities go: Your code should expect the rest of the codebase to be implemented per the specification, but it should also be prepared to handle things gracefully when the rest screws up. You are not responsible for testing other code than your own, but you are responsible for making your code resilient to misbehaving code on the other end, and you are also responsible for testing your code's resilience. That's what the third test above does.

2
  • did you read the comments below the question? The OP wrote "tests", but he meant it in the sense of "validation checks" and/or "exception handling"
    – Doc Brown
    May 24, 2013 at 10:53
  • 1
    @tdammers: sorry for the misunderstanding, I meant in fact exception handling.. Thanks anyway for the complete answer, the last paragraph is what I was looking for.
    – rdurand
    May 24, 2013 at 12:23
1

There are 3 main principles I try to code by:

  • DRY

  • KISS

  • YAGNI

The rub off all these is that you risk writing validation code that is duplicated elsewhere. If the validation rules change, these would need to be updated in multiple places.

Of course, at some point in the future, you might replatform your database (it happens) in which case you might think having the code in more than one place would be advantageous. But...you're coding for something that may not happen.

Any additional code (even if it never changes) is overhead as it will need to be written, read, stored and tested.

All the above being true, it would be remiss of you to do no validation at all. To display a full name in the application, you would need some basic data - even if you don't validate the data itself.

1

In layman's words.

There is no such a thing as "the database" or "the application".

  1. A database can be used by more than one application.
  2. An application can use more than one database.
  3. The database model should enforce data integrity, that includes throwing an error when a a required field is not included in an insert operation, unless a default value is defined in the table definition. This must be done even if you insert the row directly into the database bypassing the app. Let the database system do it for you.
  4. Databases should guard data integrity and throw errors.
  5. Business logic must catch those errors and throw exceptions to the presentation layer.
  6. Presentation layer must validate input, handle exceptions or show a sad hamster to user.

Again:

  • Database->throw errors
  • Business Logic->catch errors and throw exceptions
  • Presentation Layer->validate, throw exceptions or show sad messages.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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