0

I have a Windows service with a fluent interface like this:

aRequest = Repository.getRequest()
                     .createProcess()
                     .validate();

Sometimes getRequest() could return a null value and this would cause an error in createProcess(). I could banally split getRequest() from createProcess(), but if I wouldn't do that what way should I follow, what way is better:

  • Check if request (this) is null and in the case return null:

    if(this is null)
      return null
    

    I could do this check in every method next to getRequest(). At the end aRequest will be null.

  • Throw an exception if createProcess() method receive a null value:

    if(this is null)
       throw new NullRequestException();
    

PRO of the second way: Only second method need a check, independently of the number of method in the chain.

CON of the first way: Every method in the chain needs a check

Now the question: Is second way a bad use of exception concept, since could be normal the absence of request sometimes?

BAD_SEED
  • 257
  • 2
  • 11
  • 2
    I'd say the answer is "neither", because [`null` is considered harmful](http://programmers.stackexchange.com/q/12777/116461) and [there are better alternatives](http://code.google.com/p/guava-libraries/wiki/UsingAndAvoidingNullExplained). If an `Optional` type isn't your cup of tea, then throw an exception from the `getRequest` method. – Doval Apr 30 '14 at 15:09

3 Answers3

2

If you want to maintain the fluent syntax, getRequest() must never return null! The fluent interface must return this at every step, and this can't be null. ;)

If the internal call inside getRequest returns a null, then you need to change the state of Repository so that validate() can pick up that condition and throw an exception or handle it appropriately. You could have a member variable "String answer;" and set & check that for null, or else have a "boolean nullAnswer;" member.

Also, take a look at Guava's "Optional" class, because it's a great way of handling values that might not be present. Even then, make sure your fluent methods always returns this (or the variant return new Respository(...)).

sea-rob
  • 6,841
  • 1
  • 24
  • 47
1

Is it possible to not return null from getRequest(), or at least have a separate version of it doing so? If it threw an exception instead, and other parts of the chain would do the same, you'd have a nice consistent error handling.

What a pity that both Java and C# lack the Maybe and Either idioms; Optional has seen some adoption, though. Returning null is a bad idea: it hides the source of your problem. It is infuriating during troubleshooting. In general, fail fast, right where the problem occurs. I'd use exceptions.

9000
  • 24,162
  • 4
  • 51
  • 79
0

I think it is fine that getRequest returns null, so long as the reader knows that it can.

The problem of your example, the way it's written, it reads as it should 100% of the time it will create a process.

If the expectation is that getRequest should never return null, then that method should be the one throwing the exception. If it's expected that getRequest returns null instead, you should modify your usage of it to handle that case.

private Request createProcessIfRequestFound() {
    var request = Repository.getRequest();

    if (request != null) {
        request.createProcess().validate(); 
    }

    return request;
}

My personal preference is to avoid fluent interfaces, in my opinion, ironically, I find they are less fluent than writing code non-"fluently"

Matthew
  • 1,976
  • 3
  • 18
  • 26