Return operation “status” from a method

I sat with Mario today in order to figure out the best way to receive a “status” message from one of our business layer methods.


The task was clear:

We want to activate a rules validation method which will query the data and retrieve the missing operation(s) which are needed before the user can save his data. It is possible that the user have only one operation he has to do but it’s also possible that he has to do N operations. We needed some sort of status combination.
In addition, if the validation passes – we want some sort of “Everything is OK” status.

Our options:



  1. Use ref\out parameter and fill a string variable. Concat error messages to that string and send it back to the caller. The method will return true if everything is good and false otherwise.
  2. Return the error message(s) as string. Again, we’ll have to concat the error messages.
  3. Return a Bitwise Enum which will allow us to “add”\”remove” statuses.

Elimination process:


The main reason we quickly drop options (1) and (2) is the orientation of the error message(s). Sometimes, in act of pure laziness, we will print the error we’ve got from the BL method directly to the user screen. Some time passes by and the user gives you a call and says “Hey, I’m trying to save a user but it throws me an error like [Rule 1] is invalid… What the heck is [Rule 1] ?? I don’t remember seeing it in the user manual!” and you’ll reply “Wait a second, let me see… Hmm… if… else…not… Oh ya!! the bank account number you filled doesn’t match the bank address !”. See the problem here ?


Think about it – your business layer should return a “programmer oriented” message rather than “client oriented” message. If you’ll return some sort of client oriented message from your BL – how would this work in multilingual application ? your BL would talk with some Resource Manager just to return the “correct” message to the user ? No. This is why you have Graphic User Interface layer. So we’ll send a programmer oriented message from the BL back to our caller (for logging purpose), but wait, this will require our GUI to parse the string we’ve got from the BL method and format it for the client. Something like:


if (errorMessage.indexOf(“Rule 1 is invalid”) != -1 && errorMessage.indexOf(“Rule 2 is invalid”) != -1)
{
    // show “1). You must fill the user details. 2). You must fill the user paycheck”
}
else if (errorMessage.indexOf(“Rule 1 is invalid”) != -1)
{
    // show “You must fill the user details”
}
else if (errorMessage.indexOf(“Rule 2 is invalid”) != -1)
{
    // show “You must fill the user paycheck”
}
else
{
    // show “Save complete”
}


Yes, I can make some refactoring (constants, ToLower(), split) but no matter what I’ll do – this code smells (terrible) !


Solution:


This leaves me with option 3. It would be great to ask something *like*:


if (status == ActionStatuses.InvalidRule1 && status == ActionStatuses.InvalidRule2)
{
   // show “You must fill the user details. You must fill the user paycheck”
}
else if (status == ActionStatuses.InvalidRule1)
{
   // show “You must fill the user details”
}
else if (status == ActionStatuses.InvalidRule2)
{
   // show “You must fill the user paycheck”
}
else
{
   // show “Save complete”
}


It’s time to write some code, shall we ?


1). Let’s define a bitwise Enum, so will be able to create a status combination:


[Flags]
public enum SaveUserStatuses
{
   OK = 1,
   BankAccountMissing = 2,
   WrongEmailFormat = 4,
   BankAccountMismatchBankAddress = 8,
   SaveFailed = 16
}


The only rule: if you need to add more values just double the previous value by 2.


2). In our business layer method, we’ll do something like:


public static SaveUserStatuses Save(User user)
{
   SaveUserStatuses status = SaveUserStatuses.OK;


   if (!CheckIfBankAccountExists(user.BankAccount))
      status = (status | SaveUserStatuses.BankAccountMissing) & ~SaveUserStatuses.OK;

   if (!CheckEmailFormat(user.Email))
      status = (status | SaveUserStatuses.WrongEmailFormat) & ~SaveUserStatuses.OK;


   // you get the idea…

   if (status == SaveUserStatuses.OK)
   {
      if (!UsersDal.Save(user))
      {
         status = SaveUserStatuses.SaveFailed;
      }
   }

   return status;
}



The trick here is for every bad validation you add (“|” – bitwise OR)  the required status and remove the “OK” status by using “~” complement operator.


3). Finally, in our GUI:


SaveUserStatuses status = UserBL.Save(someUser);

if ( (status & SaveUserStatuses.BankAccountMissing) == SaveUserStatuses.BankAccountMissing &&
     (status & SaveUserStatuses.WrongEmailFormat) == SaveUserStatuses.WrongEmailFormat )
{
    // show the required message – client\user oriented !
}
// like the above – parse the status and show the required client oriented message(s).



This code smells a lot better:


a). We have no problem extending this code: (1) Add another member to the Enum (2) Add another check to our BL (3) handle the new status at GUI level.


b). We don’t have to parse strings in order to build our errors. Parsing strings is error prone. Using Enum values can set a contract between the BL and the GUI. If someone will break it, we’ll get compile time error (fail fast).


Back to code…

 

Oren Ellenbogen

 

5 thoughts on “Return operation “status” from a method

  1. I think this is not a good solution to this problem.

    "Think about it – your business layer should return a "programmer oriented" message rather than "client oriented" message."

    Once you provide a "programmer oriented" message to the caller which is then translated to a "client oriented" message you are creating duplication and breaking encapsulation, whether you’re using strings or enums.

    "We have no problem extending this code: (1) Add another member to the Enum (2) Add another check to our BL (3) handle the new status at GUI level."

    This is a huge problem. Every change requires coordination of an unknown number of areas in the code (depends on how many UI elemnts use the ‘business’ object).

    "your BL would talk with some Resource Manager just to return the "correct" message to the user ?"

    You’re half-way there – you should try to find a way the business class can provide the UI with the final error message without using literal strings (which may cause the multilingual problem).

    The solution to this problem should include a one-step process of adding a future error condition. Put another way, the ‘knowledge’ of a single error condition should appear in one place in the code.

  2. I guess that Yoni has some point, though I’d like him to add a more detailed example of achieving the goal of "the ‘knowledge’ of a single error condition should appear in one place in the code".

    I’d like to comment about the bitwise enum straucture.
    The idea of using bitwise flags variable is good, and it’s c# implementation quite clean, but there should be one more thing to take care of when using it.
    You should make sure that the user won’t be able to "ask" logical questions that are legal, but has no sense in them.
    let’s say the enum flag is declared as in your code, the next code is legal syntaxly:

    SaveUserStatuses mySaveUserStatuses = 17;
    if (mySaveUserStatuses == (SaveUserStatuses.OK & SaveUserStatuses.SaveFailed))
    // some weired piece of business login here

    you should do your best to avoid possible conflict between flag statuses.
    I’d define the OK to equal to 0, so any bit of the variable being set to true, would cause it to return false for SaveUserStatuses.OK.

  3. The goal of "the ‘knowledge’ of a single error condition should appear in one place in the code" is achieved by recognizing the class responsible for defining the error condition and providing it with the means to notify the user of the error condition.

    In the article’s example I would recognize that the static method ‘Save’ is responsible for defining possible error conditions and provide it with an ErrorReporter object capable of reporting the errors to the user. This would meen that whenever a new error condition is introduced to this process, the ‘Save’ method is the only location requiring maintainance.

  4. Why not return a List<SaveUserStatuses> instead of bitwise magic? Then if the list.Count == 0, it’s all good, else you have the enums one-by-one to enumerate and (hopefully) look up meaningful (hopefully) localized (hopefully) user-oriented messages to display, while still being able to do switch-based logic on the enumeration values.

  5. Marc,

    The only benefit of using an enum is performance.
    Returning a List from a remote object causes a large amount of bytes to be passed between tiers.
    I’d use the ErrorReporter, as Yoni suggested. It’s a clean object, and its name declares its purpose.

Comments are closed.