C# Design Unit Testing – Better Approach to Write Methods Returning Different String Outputs

cdesignunit testing

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;
        }

Best Answer

I would suggest to introduce a special result type, something along the lines of

class RentalCheckResult
{
     public enum CheckState 
     {
         PaymentUnclear, 
         CreditLimitReached,
         BookNotAvailable,
         QuotaExceeded,
         Ok
     }

     public CheckState State {get;private set;}

     private int NoOfBooks;
     
      // "noOfBooks" currently is only used for QuotaExceeded,
      // but introducing an extra subclass just for this state,
      // (or for every CheckState) seems overdesigned.
     public RentalCheckResult(CheckState state, int noOfBooks=0)
     {
        State=state;
        NoOfBooks=noOfBooks;
     }

     public override string ToString()
     {
         switch(State)
         {
              case PaymentUnclear:
                   return "Your payment is not clear.";
              case CreditLimitReached:
                   return "Your credit limit is over";
              case BookNotAvailable:
                   return "Book not availble."
              case QuotaExceeded:
                   return $"You have already rented {NoOfBooks}.books";
              default:
                   return "";

         }
     }
}

I guess the usage in BookRentCheck is clear, it needs to return a RentalCheckResult object instead of a string. This will make it possible to write unit tests for BookRentCheck which are independent from spelling corrections or translations.

RentalCheckResult itself is simple enough that it does not require any unit tests for itself. If it seems necessary, the enum can be replaced to a class hierarchy with subclasses RentalCheckResultPaymentUnclear, RentalCheckResultCreditLimitReached, and so on, where NoOfBooks will only exist as a member of RentalCheckResultQuotaExceeded.

Related Topic