CODEWITHSUNDEEP
oopvalidationdesign-patterns
Dmytro Krasun
Dmytro Krasun

Reputation: 1742

Validation in setters: What approach is better?

I have dispute with my friend.

He said me that this code: 

method SetBalance(balance) {
    if (balance > 0) {
        this.balance = balance;
        return true;
    }
    return false;
}

is better then this: 

method SetBalance(balance) {
    if (balance < 0) {
        throw new InvalidArgumentException("Balance couldn't be negative")
    }
    this.balance = balance; 
}

My question is: "Which approach is better for validation?" and why?

Thank you.

Upvotes: 4

Views: 263

Answers (6)

theD
theD

Reputation: 135

Exceptions handling is the method of choice to handle errors in OOP, whether it is runtime error or logic error. One of the arguments to justify exception handling is that, if error code is used a client of a function may forget to check the error code leading to a potential bug which may not be easily found. Whereas, if exception handling is used, even if it is not caught, it will propagate until it is handled. So, OOP purists always advice using exception handling.

Upvotes: 0

Ali Ferhat
Ali Ferhat

Reputation: 2579

My personal opinion: both approaches has its uses. It depends on why the situation arose, could it be prevented, what the client could have done to prevent it.

Server:

method SetBalance(int balance)
{
   // do something only if balance >= 0
}

Client:

myServer.SetBalance(balance);

Case 1: The situation arises only because of a bug somewhere (in server or client). Throw exception. Let the responsible party fix its code.

Case 2: A bad input is received from some external dependency. The client could easily check the precondition. Throw exception, let the client fix its code if she is unhappy with that.

if(balance>0)
  myServer.SetBalance(balance);
else
  //take appropriate action.

Case 3: A bad input is received from some external dependency. The client could not easily check the precondition. Example: it would require the client to parse a string, which is the server's job. Here, it makes sense that the server should return success:

bool succeeded = myServer.SetBalance(balance);
if(!succeeded)
  //take appropriate action.

After writing my answer down, I noticed it all boils down to this: The client should not have to use try-catch to call the method SetBalance. (Either it's a bug so don't catch the exception, or check the precondition yourself, or inspect the return value)

Here is a good reading on a related subject: http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx

Edit: Jojo makes a good point. Rename the method TrySetBalance() if there is a probability that it might fail.

Upvotes: 0

Jojo
Jojo

Reputation: 2760

Besides the answers given already, returning a boolean value in a function named "setFoo" is very misleading. If you translate a boolean value you come up with yes/no. So a function that returns booleans in oop should be readable as a question that gives yes/no as an answer, e.g. isBalanceSet.

In discussion with collegues we considered the only valid return-value for a normal setter is the class itself for fluent-interfaces.

Regarding your question throwing exceptions is the better solution if the class actually validates its properties itself ;)

Upvotes: 0

Roman Saveljev
Roman Saveljev

Reputation: 2594

Return value is easily ignored. Here you have clear coding bug, so crash loud. I would even halt/segfault/stop program - the exception may still be caught. We live in "ideal world", but imagine our less fortunate colleagues - they might well go for gagging error signalling facility being unable to deal with the root cause (insert here more complicated error scenario). Coding gets much easier when you dont give clients ways to do things wrong way.

Upvotes: 1

sundar
sundar

Reputation: 1760

In case of setters, developers will never expect the return value from those methods since they feel that this will anyway will take care of setting the value.
In case, if there is an exception, then this will help then to understand that something is really wrong and look after the code again.
Instead of runtime exception, you better define your own exception like InvalidBalanceException and specify that this method will throw this exception in the signature itself. This avoids surprises at testing phase and decide the business logic in development phase itself.

Upvotes: 1

Luchian Grigore
Luchian Grigore

Reputation: 258598

Ah. Return status versus exception.

I prefer throwing exceptions, because you can't really ignore an exception. The program either crashes, or you have to explicitly catch and handle it.

You can just ignore a return status.

Besides, isn't this the reason exceptions exist in modern programming? To deal with, well... exceptions (balance <= 0, something that shouldn't happen).

Upvotes: 6

Related Questions