0

In our codebase I saw a method that is similar to following:

public void doSomething() {
    try {
        ...
    } catch (Exception e) {
        ...
        //log something
        ...
    }
}

The caller has no way to know if the method did what it is supposed to. Are there any general Java guidelines on not writing void methods that do not throw exceptions?

  • 1
    What makes you think that the method is not unit testable? Can you edit your question to provide the part of the code which cannot be tested? – Arseni Mourzenko Jun 25 '15 at 21:44
  • 1
    Depending on the unit test framework in use, you can generally write unit tests that assert if a method throws. Since the method returns void, presumably the method is causing side-effects; you can make assertions based on those side-effects occurring. – Robert Harvey Jun 25 '15 at 21:48
  • 1
  • @gnat: This function doesn't return anything, nor is it swallowing the exception (in the anti-pattern sense). – Robert Harvey Jun 25 '15 at 21:49
  • @MainMa: I am using JUnit 4. How would you assert if it returns void. How would you expect an exception if it does not throw any. – hrishikeshp19 Jun 25 '15 at 21:51
4

First of all this method is not unit testable.

There is nothing in the code you quoted which prevents it from being covered by unit tests.

The presence of try/catch is irrelevant:

  • You can test the situation where there is no error and check whether the side effects of this method correspond to the expectations.

  • You can also test the situation where an exception is produced, and ensure that the expected message is indeed logged (hopefully, the logging framework can be replaced with a mock.)

This means that you can get a 100% branch coverage despite the global catch of the exception. Talking about the global catch, this is not something you really want in your code base (unless there is a throw inside a catch), but this has nothing to do with unit tests.

This would apply as well if the method was catching only a specific exception, say FileNotFoundException, but there could be other exceptions thrown, such as StackOverflowException. You can then have three tests:

  • The ordinary situation with expected side effects,

  • The situation where FileNotFoundException is thrown and the expected log message is added,

  • The situation where StackOverflowException is thrown; any decent unit testing framework allows you to test that the exception is effectively thrown in a tested case (and the test will fail if the method doesn't throw the expected exception).

Are there any general Java guidelines on not writing void methods that do not throw exceptions?

By “do not throw exceptions”, I suppose you rather mean “catch all the exceptions”.

  • Catching exceptions has nothing wrong as soon as the method knows how to deal with the exceptional cases. For instance, a method which accesses the configuration file and encounters FileNotFoundException may fallback to default configuration instead, without reporting any exception to the caller. This is fine.

  • Catching exceptions globally (that is catch (Exception)) is slightly more problematic. Cases where methods are able to deal with any exceptional situation are very rare. There are still two exceptions:

    1. A method can catch any exception, and then throw the same exception (preferably without breaking the stack). This, for instance, enables logging: a method logs an exception, and then lets the caller deal with it.

    2. A method which is on top of the stack can catch any exception, because the choice is rather limited: either you catch the exception, or you let the program crash. The developers may prefer in this case to catch everything, even exceptions they can't deal with, and, for instance, present the user with a user-friendly dialog box which makes it possible to report the exception to developers' team.

  • Are there any general Java guidelines on not writing void methods that do not throw exceptions? (Yes/No) – hrishikeshp19 Jun 25 '15 at 21:54
  • By “do not throw exceptions” I meant exactly that. Please read what bryancook's answer says: "If a method can't do it's job, it should throw an exception." Do you think that a valid statement? – hrishikeshp19 Jun 25 '15 at 22:09
2

Generally speaking, an Exception is suited for this and the best practice you should follow is: "If a method can't do it's job, it should throw an exception."

Understandably, you may want execution flow to continue without exceptions. The .NET space handles this fairly well and provides a convention for methods that don't throw exceptions. Methods that start with "Try" always return a boolean and do not throw exceptions.

The benefit of this approach is that you can write tests on the DoWork method to test that exceptions are thrown when expected, and conversely no exceptions are thrown when calling the Try equivalent.

public void DoWork() {
   // try to do the work, and if it fails, throw
}

public bool TryDoWork() {
   try {
      DoWork();
      return true;
   } 
   catch {
      return false;
   }
}

Alternatively, if that's not your cup of tea, another approach is to mock out your Logger and write tests that capture that errors are logged.

Two supporting examples:

  1. Clean Code - by Robert C Martin
  2. Working Effectively with Legacy Code - by Michael Feathers
  • Thanks BraynCook! :) I agree with you. Can you give me any references to "If a method can't do it's job, it should throw an exception."? Thanks! – hrishikeshp19 Jun 25 '15 at 22:00
  • Thanks! Your answer is already great. Just in case if you have online resources, that would be awesome. – hrishikeshp19 Jun 25 '15 at 22:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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