Simple refactoring example

Question

Given the following code:

public static bool UpdateUser(string userId, 
                                      string jobTitle)
{    
        return GetProvider().UpdateUser
            (userId, jobTitle);
}

It needs to be changed and may not return a bool, for example:

    UserProfile userProfile = new UserProfile();
    userProfile.Initialize(user.UserName, true);
    userProfile.ProfileJobTitle = jobTitle;
    userProfile.Save();

Should you ensure it does return a bool or just change the method completely?

What's the right approach to this type of problem?

Answer 1

I believe is up to you and what you are trying to do, If you just don't mind about the returned value (bool) you can ignore it, but if you mind about it let's say if it returns false you could display a message (i.e. "Save was not successful" or whatever).

Answer 2

Presumably the second block of code in your question is an implementation of GetProvider::UpdateUser().

The obvious alternative to ensuring that it does return a bool would be to have it throw an exception on failure. Depending on how much existing code you have that does/doesn't return a value, this might save a fair amount of work. In particular, anything that currently returns without a value, can be left alone. Anything that returns "success" can simply have the return value removed. The things that return failure will need modification to throw an exception instead.

Should you do this, the difficult part will be ensuring that all the other code is exception safe. Depending on your other use of exceptions (if any), you should almost certainly do this regardless, but this may force the issue, so you'll need to clean up problems in the other code sooner rather than later.