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?

Was it helpful?

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top