Returning status codes from business layer

This is what I've been doing so far in my business layer to return error messages to the user.

public void CreateMerchant(MerchantCreationModel model, out MerchantCreationStatus status)
{
    //check if merchantId exists
    MerchantDTO merchant = _merchantRepo.GetMerchant(model.MerchantId);

    if (merchant != null)
    {
        status = MerchantCreationStatus.MerchantAlreadyExists;
        return;
    }

    // Rest of merchant creation logic has been removed for clarity

    status = MerchantCreationStatus.Success;
}

I have a ErrorCodeToString() method in my controller that gets the text to display.

What do you think of this technique?

Status codes have long gone the way of the Dodo --- made extinct by exceptions, actually.

The problem with a status code is that it can be ignored. An exception cannot. Consider these two examples.

Status Codes as Return Values

First, the "merchant repository" implementing CRUD operations for merchants:

public enum MerchantStatusCode
{
    OK = 0,
    Created = 1,
    Updated = 2,
    Deleted = 3,
    MerchantAlreadyExists = 4,
    MerchantDoesNotExist = 5
}

public class MerchantRepository
{

    public MerchantStatusCode Create(Merchant model)
    {

        if (MerchantExists(model))
            return MerchantStatusCode.MerchantAlreadyExists;

        // Save to DB

        return MerchantStatusCode.Created;
    }
}

Now, the C# code to create a merchant and ignore error handling:

var merchantRepository = new MerchantRepository();

// Create a merchant. I'm not really sure what could go wrong, and if
// something should go wrong, I'm not really sure if it's a problem.
merchantRepository.Create(merchant);

The MerchantRepository.Create method returns a status code that our programmer promptly ignores. By looking at these lines of code, there is no indication that you should do any error handling before subsequent lines of code are executed.

Status Codes as Output Parameters

Let's also say an output parameter is used:

public class MerchantRepository
{
    public void Create(Merchant model, out MerchantStatusCode statusCode)
    {

        if (MerchantExists(model))
        {
            statusCode = MerchantStatusCode.MerchantAlreadyExists;
            return;
        }

        // Save to DB

        statusCode = MerchantStatusCode.Created;
    }
}

And the code to ignore the status code:

var merchantRepository = new MerchantRepository();
MerchantStatusCode code;

// Create a merchant. Sure I could check the "status" but, meh. If
// something fails it must not be THAT important. After all, good
// programmers are lazy, right?
merchantRepository.Create(merchant, out code);

While this exhibits more of a code smell, an error code doesn't halt program execution.

Now, let's tweak this to throw an exception:

Using Exceptions

public class MerchantRepository
{
    public void Create(Merchant model)
    {
        if (MerchantExists(model))
            throw new RecordExistsException();

        // Save to DB
    }
}

public class RecordExistsException : Exception
{
    ...
}

And the C# code to use this in the same manner as the previous example where the programmer blissfully ignores problems:

var merchantRepository = new MerchantRepository();

try
{
    merchantRepository.Create(merchant);
}
catch Exception
{
    // I realize this method throws exceptions when it fails to create
    // a merchant, but I am a complete idiot as a programmer, and
    // failure as a human being, so I'm going to blindly swallow any
    // possible errors, then move to another team or job and let you
    // try tracking down this production problem for 6 months while
    // the company bleeds customers and your boss screams at you so
    // hard he spits his bagel and coffee all over your face.
    //
    // But, hey. At least I didn't write "TODO: Error handling"!
}

Both examples work the same when the code is executed and a duplicate merchant is created, but there is one glaring difference here: The empty try-catch block. Empty try-catch blocks go beyond "code smells" to "steaming piles of rotting flesh that have been baking in the sun all afternoon." I get an ulcer every time I see this.

Now, if we tweak example #2 to be the exact same code as example #1:

var merchantRepository = new MerchantRepository();

// Even though I don't handle an exception, at least the next
// line of code won't execute if something goes terribly wrong.
// I'm not an idiot. I just HAVE to create a merchant here, and
// failure to do so is a hard stop.
merchantRepository.Create(merchant);

No subsequent lines of code will get executed, and the programmer does not have to guard against this condition unless they want to.

The lesson here: For the love of God do not blindly trap exceptions and do nothing, unless you document this behavior in comments and provide a really really really really really really really really really really really really really good reason for doing so.

Regarding your particular approach, there are a couple things that you should be doing instead:

Error Handling Best Practices Regarding Database Operations

• Prior to creating a merchant, check the database to ensure the merchant doesn't already exist. This is basic data validation, and should be handled in its own layer.

• When creating a merchant, if a merchant happens to exist, throw an exception which will force you, as a programmer, to handle this "exceptional situation" (hence the term Exception).

When a database operation is invoked, it needs to assume that all data is valid, and then throw an

StackExchange Code Review Q#63049, answer score: 35