Which pattern to select when trying to retrieve success/failed and a object from a method?
-
14-11-2019 - |
Question
I'm discussing different implementing techniques with a colleague
Alternative A
User user;
if (users.TryGet(1, out user))
Console.WriteLine(user.ToString());
else
Console.WriteLine("Failed to get user..");
Alternative B
ResultSet<User> result = users.Get(1);
if(result.OK)
Console.WriteLine(user.ToString());
else
Console.WriteLine("Failed to get user..");
Alternative A is similar in behaviour as int.Parse(string, out value)
and is easy to extend and read. Adding more objects to the output when success seems easy and readable.
Alternative B seems a bit more convoluted in that there is more code on each line. But can be extended to include other information in the ResultSet
as well as the object back. Is it more difficult to return more than one object or any ideas regarding workarounds?
Is it considered ugly/bad practice to use out parameters like that?
Solution
In general, Microsoft discourages using out-parameters in public methods. A proper approach would be to throw an exception if a method fails to do the job identified by its name. In this case, if the Get
method is unable to Get the data record, there should be an exception. If this situation is occuring very frequently, you could use Alternative B to improve performance a little bit in contrast to throwing exceptions every time.
As for Alternative A, method should be named TryGet
in this case to be consistent with FCL naming conventions (TryParse
, TryGetValue
etc.).
OTHER TIPS
The problem with both of those methods is that you don't know why the user failed to be retrieved.
For example, if your Get
or TryGet
methods look like this...
public bool TryGet(int userId, out User user) {
try {
var conn = CreateDbConnection();
var myUser = GetUser(userId, conn);
user = myUser;
return true;
}
catch {
return false;
}
}
... then you have no way of knowing what failed while retrieving the user. Maybe the DB connection couldn't be created, maybe no user exists with the specified ID, etc.
A better method might be this:
public MyUser GetUser(int userId) {
MyDbConnection conn;
try {
conn = CreateDbConnection();
}
catch (Exception ex) {
throw new Exception("The DB connection could not be created.", ex);
}
MyUser myUser = GetUser(userId, conn);
if (myUser == null)
throw new Exception("No user exists with the ID '" + userId + "'.");
return myUser;
}
That way, you know what's failing, and you have a better message to log further up the chain.