Which one is a better practice in programming?
I am not talking about complete exclusivity. It's more for things like:
list<T>.Find, where you get default(T) or your value, instead of ValueNotFound exception (example).
or
list<T>.IndexOf, where you get -1 or the correct index.
Well, the answer is it depends.
In the case of where an item is not found in a list, throwing an exception is a horrible practice, because it's completely feasible that the item might not be in the list.
Now if this was a specialized list of some sort, and the item should absolutely be found in the list, then that should throw an exception, because you have encountered a situation which was not feasible/reasonable.
Generally speaking, for things like business rules and the like, specialized error codes are better, because you are aware of the possibility of these things happening, and want to react to those possibilities. Exceptions are for the cases that you don't expect, and can't continue with code execution if they occur.
From purely design perspective I prefer throwing exceptions when an "exceptional" thing happens, e.g. the key you asked for was not found. The main reason is that it makes life easier for consumer - they don't have to check the value after each call to your function. I also like the TrySomething functions, because then the user can explicitly test if the operation was successful.
Unfortunately exceptions in .Net are quite expensive (from my experience it takes usually about 50ms to throw one), so from practical perspective you may want to return one of these default values instead of throwing an exception, especially in GUI programming.
I think it's slightly situational, but more governed by whether the return event is actually logically an exception or not. The indexOf example is a perfectly normal output response where -1 is the only thing that makes sense (or null for ref types).
An exception should be exactly that: exceptional, which means you want all the stack tracing and handling that you can get from a real exception.
In cases you mentioned I'd prefer return value as I know right there what to do with that. And there is no reason to bother with catch in the same scope.
Of course if your logic is built in a way that the values always should be in the list and their absence is the programmer logic error - exceptions are the way.
Coming from a Java background, I prefer exceptions in most cases. It makes your code much cleaner when half of it isn't spent checking return values.
That said, it also depends on the frequency that something is likely to result in a "failure". Exceptions can be expensive, so you don't want to be throwing them around needlessly for things that will frequently fail.
An exception should be something "exceptional". So if you call Find, and you expect to find something, no matter what, then throwing an exception if you do not find something is good behavior.
If however its the normal flow, that you sometimes do not find something, then throwing an exception is bad.
More Effective C# by Bill Wagner made an excellent recommendation in regards to exceptions. The thought is to go ahead and throw an exception when performing an operation, just make sure that you provide hooks in order to check if the value will throw an exception.
For example
// if this throws an exception
list.GetByName("Frank") // throws NotFound exception
// provide a method to test
list.TryGetByName("Frank") // returns false
that way you can opt out of the exception by writing something like
MyItem item;
if (list.TryGetByName("Frank"))
item = list.GetByName("Frank");
A rule of thumb is to use exceptions only when something happens that "shouldn't happen".
If you would expect that a call to IndexOf() might not find the value in question (a reasonable expectation), then it should have a return code for that (probably -1 as you say). Something that should never fail, like allocating memory, should throw an exception in a failure case.
Another thing to remember is that handling exceptions is "expensive" in terms of performance. So if your code regularly handles exceptions as part of normal operations, it won't perform as fast as it could.
You may enjoy my two-part blog series that discusses a lot of trade-offs here depending on what features your programming language supports, as it seems quite relevant:
An example of the interplay between language features and library design, part one
An example of the interplay between language features and library design, part two
I'll add that I think a lot of the answers to this question are poor (I downvoted many of my cohorts). Especially bad are APIs along the lines of
if (ItWillSucceed(...)) {
DoIt(...)
}
which create all kinds of unhappy issues (see my blog for details).
As in many issues related to programming it all depends...
I find that one really should first try to define your API so exceptional cases can not happen in the first place.
Using Design By Contract can help in doing this. Here one would insert functions that throw an error or crash and indicate a programming error (not user error). (In some cases these checks are removed in release mode.)
Then keep you exceptions for generic failures that can not be avoided like, DB connection failed, optimistic transaction failed, disk write failed.
These exceptions should then typically not need to be caught until they reach the 'user'. And will result in the user to need to try again.
If the error is a user error like a typo in a name or something then deal with that directly in the application interface code itself. Since this is then a common error it would need to be handle with a user friendly error message potentially translated etc.
Application layering is also useful here. So lets take the example of transfering cash from one account an other account:
transferInternal( int account_id_1, int account_id_2, double amount )
{
// This is an internal function we require the client to provide us with
// valid arguments only. (No error handling done here.)
REQUIRE( accountExists( account_id_1 ) ); // Design by contract argument checks.
REQUIRE( accountExists( account_id_2 ) );
REQUIRE( balance( account_id_1 ) > amount );
... do the actual transfer work
}
string transfer( int account_id_1, int account_id_2, double amount )
{
DB.start(); // start transaction
string msg;
if ( !checkAccount( account_id_1, msg ) ) return msg; // common checking code used from multiple operations.
if ( !checkAccount( account_id_2, msg ) ) return msg;
if ( !checkBalance( account_id_1, amount ) ) return msg;
transferInternal( account_id_1, account_id_2, amount );
DB.commit(); // This could fail with an exception if someone else changed the balance while this transaction was active. (Very unlikely but possible)
return "cash transfer ok";
}
I've read somewhere a nice rule about this that I like very much. It says - "a function should throw an exception if and only if it cannot perform the task it was meant to".
So what I usually do is to decide what a function should do (that usually comes from business requirements or something), and then throw exceptions for everything else.
If you have designed your application well, your functions will be pretty small and perform relatively simple tasks with simple return values. Deciding on exceptions by the above rule will not be difficult.
Of course, there are always ambiguous cases (like with a key not found in a dictionary). Those should be far and inbetween, but there you'll just have to use your gut feeling on what is the more elegant solution.
Oh, and with all this never forget: for this to work well an nice only catch exceptions that you can process. Mostly that means you will catch them only in the upper UI levels where you can display the, to the user and log them or something. Lower levels might use finally blocks or rethrow exceptions after some processing of their own, but genuine caught exceptions in low levels usually indicate bad design.
Which would you rather use?
A:
item = list.Find(x);
B:
If (list.Contains(x))
item = list.Find(x);
else
item = null;
C:
try {
item = list.Find(x);
}
catch {
item = null;
}
I'm willing to bet the answer is A. Therefore, in this case returning Default(T) is the right choice.
Better languages let you do whatever fits your needs. Like in Smalltalk-80:
The following will raise an exception if there's no user for the id:
user := userDictionary at: id
This one will evaluate the given Block which is a high level function:
user := userDictionary at: id ifAbsent: [
"no such id, let's return the user named Anonymous"
users detect: [ :each | each name = 'Anonymous' ] ]
Please note that the actual method is at:ifAbsent:.
For the first, I don't really like the default(T) option: what if you have an int list where 0 (presumably this is int's default; I don't use C#) is a perfectly allowable value?
(Apologies for the Java syntax, but if I tried to guess C# I'd probably make a mess of it somewhere :) )
class Maybe<T> {
public Maybe() {
set = false;
value = null;
}
public Maybe(T value) {
set = true;
this.value = value;
}
public T get(T defaultVal) {
if (set)
return value;
return defaultVal;
}
private boolean set;
private T value;
}
Then of course Find would return a Maybe<T>, and the caller chooses some value that's a sensible default in this context. Perhaps you could add getDefault as a convenience method for when default(T) is the right answer, of course.
For IndexOf, though, -1 is a sensible value for this, since valid values are obviously always >= 0, so I'd just do that.
what i usually do in this case is to define an Option class (inspired by F#) and extend IEnumerable with a TryFind() method that returns the Option class.
public class Option<T>
{
string _msg = "";
T _item;
public bool IsNone
{
get { return _msg != "" ? true : false; }
}
public string Msg
{
get { return _msg; }
}
internal Option(T item)
{
this._item = item;
this._msg = "";
}
internal Option(string msg)
{
if (String.IsNullOrEmpty(msg))
throw new ArgumentNullException("msg");
this._msg = msg;
}
internal T Get()
{
if (this.IsNone)
throw new Exception("Cannot call Get on a NONE instance.");
return this._item;
}
public override string ToString()
{
if (this.IsNone)
return String.Format("IsNone : {0}, {1}", this.IsNone, typeof(T).Name);
else
return String.Format("IsNone : {0}, {1}, {2}", this.IsNone, typeof(T).Name, this._item.ToString());
}
}
Then you can use it as
var optionItem = list.TryFind(x => trueorFalseTest() );
if (!optionItem.IsNone)
var myItem = optionItem.Get();
This way, whether the item exists or not, the collection is traversed only once.