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:
- 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.
- Return the error message(s) as string. Again, we'll have to concat the error messages.
- 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...