I have some code similar to shared below, which returns different kinds of response messages to the caller. If the return value is empty string, the process is continued. If a message is returned it will show the message to user. (Please consider this is demonstration code, not really used so I might have some syntax problems)
When I am writing unit tests for this code I am actually comparing the different hard coded string values with the output of the function. It makes me uncomfortable because changing the output string syntax or even fixing spelling mistakes will break my tests.
Is there a better approach to this code? Is there a better design pattern to follow? Thanks in advance.
public string BookRentCheck(string customerId, string bookId)
{
var responseMessage = "";
bool isPaymentOk = GetPaymentOk(customerId);
if (!isPaymentOk)
{
if (GetAllowedOnCredit(customerId))
{
double availbleCredit = GetAvailableCreditBalance(customerId);
double bookRent = GetRentForBook(bookId);
if (availbleCredit < bookRent)
{
responseMessage = "Your credit limit is over";
return responseMessage;
}
}
}
else
{
responseMessage = "Your payment is not clear.";
return responseMessage;
}
if (!bookAvailable(bookId))
{
responseMessage = "Book not availble.";
return responseMessage;
}
if (!bookQuotaAvailable(customerId))
{
int rentedBookCount = GetRentedBookCount(customerId);
responseMessage = "You have already rented " + rentedBookCount + ".";
return responseMessage;
}
return responseMessage;
}
return "Book not available.";
instead ofresponseMessage = "Book not available."; return responseMessage;
return "";
responseMessage
variable. Initializing to the defined/intended "nothing" value of "empty" is the right thing to do. Strings default to null, not String.Empty. Because Initializing variables is a strong signal to the maintenance programmer of intent.return responseMessage
is needed.Remove all the intermediatereturn
s. A single return is much more maintenance friendly, especially given multipleif
s with multiple lines. Now, I have used multiplereturns
but only in very short methods; this case however would not pass code review in my shop.