End method in normal flow versus exception flow

coding-styleexceptionsjava

Consider the two following examples:

public Something fetchSomething(String key) {
    if(somethingsMap.containsKey(key)) {
        return somethingsMap.get(key);
    }

    throw new IllegalStateException("key is missing");
}

versus

public Something fetchSomething(String key) {
    if(!somethingsMap.containsKey(key)) {
        throw new IllegalStateException("key is missing");
    }

    return somethingsMap.get(key);
}

Is there any general consensus which is considered "better"/more clean/preferred. Ending in normal flow or ending in exception flow.
Or is this solely opinionated?

Best Answer

Rephrased this question is make exception the de facto behaviour or make happy path the de facto behaviour the method implement.

This:

public Something fetchSomething(String key) {
    if(!somethingsMap.containsKey(key)) {
        throw new IllegalStateException("key is missing");
    }

    return somethingsMap.get(key);
}

is a fail-fast implementation.

In systems design, a fail-fast system is one that immediately reports at its interface any condition that is likely to indicate a failure. Fail-fast systems are usually designed to stop normal operation rather than attempt to continue a possibly flawed process. Such designs often check the system's state at several points in an operation, so that any failures can be detected early. The responsibility of a fail-fast module is detecting errors, and then letting the next-highest level of the system handle them.

from wikipedia.

If the question is just about the code style then it should be answered by the code style guide lines the team follows 'cause once decided after a while either way of writing the code is readable, clean, preferred.

Related Solutions

Java – Is throwing an exception an anti-pattern here

Yes, your colleague is right: that is bad code. If an error can be handled locally, then it should be handled immediately. An exception should not be thrown and then handled immediately.

This is much cleaner then your version (the getValueByKey() method is removed) :

public String getByKey(String key) {
    if (valuesFromDatabase.containsKey(key)) {
        return valuesFromDatabase.get(key);
    } else {
        String defaultValue = defaultValues.get(key);
        valuesFromDatabase.put(key, defaultvalue);
        return defaultValue;
    }
}

An exception should be thrown only if you do not know how to resolve the error locally.

C++ – Exception Hierarchy and Parsing Strings with What Message

Using exception::what() for text that is show to the end-user is indeed a problem. Suppose your project becomes a real success and you get a request to make a translation of it into some other language. That will be next to impossible if the text you show your users is directly copied from exception::what().

The correct way is indeed to create an exception (hierarchy) for the problems that your parser can encounter. For example:

class FormatError : public std::runtime_error
{
public:
  //...
  void set_line_number(int);
  int get_line_number() const;
private:
  int line_number;
};

class InvalidDataType : public FormatError
{
  // ...
private:
  String dataType;
};
// etc.

Using a common base class makes it easy to add the line number information at a later point (in ParseText) and re-throw the same exception (with throw;)

Best Answer

Related Solutions

Java – Is throwing an exception an anti-pattern here

C++ – Exception Hierarchy and Parsing Strings with What Message

Related Topic