by Zoran Horvat
While designing a complex software system we often encounter a need to pass simple, unstructured information, such as information whether operation was successful or not. Such operation might simply be designed as a method returning Boolean. Consider the following implementation of an imaginary Server class:
public class Server
{
public bool DoSomething()
{
count++;
for (int i = 1; i * i <= count; i++)
if (i * i == count)
return true;
return false;
}
private int count;
}
Object of this class is responsible to perform some operation and to return a Boolean value indicating whether operation went well or it failed. Concrete implementation returns False most of the time, and only sometimes it succeeds and returns True. This class would be consumed by some client like this:
public class Client
{
public Client(Server server)
{
this.server = server;
}
public void ExecuteComplexOperation()
{
Console.WriteLine("Preparing data...");
while (true)
{
Console.WriteLine("Calling server...");
if (this.server.DoSomething())
break;
Console.WriteLine("ERROR - will retry.");
}
Console.WriteLine("Done.");
Console.WriteLine(new string('-', 20));
}
private Server server;
}
This client class simulates some complex operation, which consists of information gathering phase and a phase in which client makes a call to the server. The tough part is that server might fail, i.e. return False. Client then chooses to retry, and it does so as many times as it is needed to the server to finally succeed. And here is the output when client is called three times in a row on the same server:
Preparing data...
Calling server...
Done.
--------------------
Preparing data...
Calling server...
ERROR - will retry.
Calling server...
ERROR - will retry.
Calling server...
Done.
--------------------
Preparing data...
Calling server...
ERROR - will retry.
Calling server...
ERROR - will retry.
Calling server...
ERROR - will retry.
Calling server...
ERROR - will retry.
Calling server...
Done.
--------------------
Now, the problem with this design is that server does not return any information about what really went wrong. It only says that operation was not successful. But, if so, then the client wants to know more because further actions might depend on precise nature of the error.
When looking at the code above, core problem is this line:
if (this.server.DoSomething())
In this precise line, the client asks for the operation outcome and only receives a True or False flag. Hence, the only thing the client can do is either to abort the operation or to repeat it. But the client is unable to print out the error code, which might be useful to the end user. What we really want is to be able to write a code similar to this:
public void ExecuteComplexOperation()
{
Console.WriteLine("Preparing data...");
while (true)
{
Console.WriteLine("Calling server...");
var status = this.server.DoSomething();
if (status.IsSuccess) // Hypothetical property
break;
Console.Write("Error #{0} occurred, do you want to retry? (Y/N) ",
status.ErrorCode); // Hypothetical property
bool retry = (Console.ReadLine() == "Y");
if (!retry)
break;
}
Console.WriteLine("Done.");
Console.WriteLine(new string('-', 20));
}
Difference is in two properties: IsSuccess and ErrorCode. IsSuccess is the True/False flag which we already have, but ErrorCode is not deducible from Boolean result in any way. And there we decide to modify the design.
The first change we are going to make in this design is to introduce a meaningful return value instead of simple Boolean result. We lean towards simplicity, but yet want to avoid being too simple. In the old days return value could be an integer number, with zero indicating success and positive value indicating an error and, at the same time, representing the error code. In modern days, it is usual to define an enumeration (with underlying type being integer) which gives type checking and meaningful names to error codes:
public enum OperationResult
{
Success = 0,
AccessDenied = 4,
DatabaseNotAvailable = 17
}
This enumeration defines two distinct error codes: 4, which is returned when client does not have privileges to access the requested resource, and 17, which is returned when there are no database connections available at the moment. We have used non-obvious values 4 and 17 just to emphasize our ability to return any integer value from the server operation. Suppose that these two error codes have been picked from a long list of codes representing errors that can be identified in the system, while only two of those errors can occur when this particular operation is executed. Modified server code may now look like this:
public class Server
{
public OperationResult DoSomething()
{
count++;
for (int i = 1; i * i <= count; i++)
if (i * i == count)
return OperationResult.Success;
if (count % 2 == 0)
return OperationResult.AccessDenied;
return OperationResult.DatabaseNotAvailable;
}
private int count;
}
Now we can rewrite the client code to utilize the added knowledge:
public void ExecuteComplexOperation()
{
Console.WriteLine("Preparing data...");
while (true)
{
Console.WriteLine("Calling server...");
OperationResult status = this.server.DoSomething();
if (status == OperationResult.Success)
break;
Console.Write("Error #{0} occurred, do you want to retry? (Y/N) ", (int)status);
bool retry = (Console.ReadLine() == "Y");
if (!retry)
break;
}
Console.WriteLine("Done.");
Console.WriteLine(new string('-', 20));
}
And here is the output produced by this modified client:
Preparing data...
Calling server...
Done.
--------------------
Preparing data...
Calling server...
Error #4 occurred, do you want to retry? (Y/N) N
Done.
--------------------
Preparing data...
Calling server...
Error #17 occurred, do you want to retry? (Y/N) Y
Calling server...
Done.
--------------------
Preparing data...
Calling server...
Error #17 occurred, do you want to retry? (Y/N) Y
Calling server...
Error #4 occurred, do you want to retry? (Y/N) N
Done.
--------------------
The end user would certainly appreciate the appearance of error code, because that code would help decide whether to proceed with the operation or not. Provided that user possesses a table with error codes, it is easy for him to decide that answer to the question should be No when error code 4 appears and Yes when error code 17 appears.
When looking just at the output, the problem seems to be resolved. But internally, the problem remains. Just take a look at the client functions which consumes the operation result:
OperationResult status = this.server.DoSomething();
if (status == OperationResult.Success)
break;
Console.Write("Error #{0} occurred, do you want to retry? (Y/N) ", (int)status);
This piece of code is an example of bad design. It relies on the fact that status is a particular enumeration type with particular meanings of different values. It would be much better to rely on properties of different OperationResult values, rather than on values themselves. For example:
OperationResult status = this.server.DoSomething();
if (status.IsSuccess)
break;
Console.Write("Error #{0} occurred, do you want to retry? (Y/N) ", status.ErrorCode);
This code is completely decoupled from the OperationResult type – be it enumeration or anything else. We are now testing whether the status means that operation was successful or not. And, if not, which code best explains the error that occurred. We will soon see how we can benefit from this design. But in the meanwhile, there is some work to do: OperationResult enumeration type does not provide properties IsSuccess and ErrorCode.
In C# it is (still) not possible to define members of enumeration type. But there is a workaround: extension methods are allowed. So we can define methods IsSuccess and GetErrorCode for OperationResult enumeration type and then use them in code:
public enum OperationResult
{
Success = 0,
AccessDenied = 4,
DatabaseNotAvailable = 17
}
public static class OperationResultExtensions
{
public static bool IsSuccess(this OperationResult res)
{
return res == OperationResult.Success;
}
public static int GetErrorCode(this OperationResult res)
{
return (int)res;
}
}
With this addition, we are able to redesign the client so that it becomes decoupled from actual values defined in OperationResult:
public void ExecuteComplexOperation()
{
Console.WriteLine("Preparing data...");
while (true)
{
Console.WriteLine("Calling server...");
OperationResult status = this.server.DoSomething();
if (status.IsSuccess())
break;
Console.Write("Error #{0} occurred, do you want to retry? (Y/N) ", status.GetErrorCode());
bool retry = (Console.ReadLine() == "Y");
if (!retry)
break;
}
Console.WriteLine("Done.");
Console.WriteLine(new string('-', 20));
}
This code provides exactly the same output as before, but it does not depend on actual implementation of OperationResult. It rather depends on qualities that some values exhibit. In that sense, OperationResult.Success exhibits quality that it represents a success, while remaining two values do not have this quality. This quality is observed by calling the IsSuccess method – in the first case it returns True, while in other cases it returns False. This is good enough for any consuming code, and there is no more reason to compare particular instance of OperationResult type with any of the predefined instances (Success, AccessDenied, DatabaseNotAvailable). OperationResult enumeration can freely be modified (new values added or existing ones removed) and client code would not have to be modified. Observe this change:
public enum OperationResult
{
Success = 0,
Warning = 1,
AccessDenied = 4,
DatabaseNotAvailable = 17
}
public static class OperationResultExtensions
{
public static bool IsSuccess(this OperationResult res)
{
return res == OperationResult.Success || res == OperationResult.Warning;
}
public static int GetErrorCode(this OperationResult res)
{
return (int)res;
}
}
In this code, we have added one value to the enumeration – Warning. This value is treated as a success. At the same time, we had to modify IsSuccess extension method to test for Warning as well. This is not the problem because IsSuccess method is actually part of the OperationResult type, although extension in its nature, but part never the less. So these two modifications in reality are just one functional modification of the OperationResult type. But once that is done, the client remains intact. Should it ever receive Warning result from the operation it calls, it would naturally treat it as a success because IsSuccess method says it to do so.
Suppose that requirements extend even further. Instead of displaying an error code, we are asked to print a user-friendly error message. It was easy to deal with error codes, because code itself was part of the enumeration value (it was the enumeration value itself). But now we want to have functionality like this:
OperationResult status = this.server.DoSomething();
if (status.IsSuccess())
break;
Console.Write("{0}. Do you want to retry? (Y/N) ", status.ErrorMessage);
This calls for a refactoring. Instead of an enumeration, we want to use a class. We will remove OperationResult enumeration definition, along with its extension methods. Enumeration will be replaced with the class with same name:
public class OperationResult
{
public bool IsSuccess()
{
return false;
}
public int GetErrorCode()
{
return 0;
}
public string ErrorMessage { get; private set; }
}
But now, we have a problem using this class. Server implementation relies on existence of several static members:
public OperationResult DoSomething()
{
count++;
for (int i = 1; i * i <= count; i++)
if (i * i == count)
return OperationResult.Success;
if (count % 2 == 0)
return OperationResult.AccessDenied;
return OperationResult.DatabaseNotAvailable;
}
This code does not compile any more. But solution is simple: we will just define read-only static fields with appropriate names. Here is the complete implementation of the OperationResult class:
public class OperationResult
{
public bool IsSuccess()
{
return this.isSuccess;
}
public int GetErrorCode()
{
return this.errorCode;
}
public string ErrorMessage { get; private set; }
private bool isSuccess;
private int errorCode;
public static readonly OperationResult Success
= new OperationResult() { isSuccess = true, errorCode = 0,
ErrorMessage = string.Empty };
public static readonly OperationResult Warning
= new OperationResult() { isSuccess = true, errorCode = 0,
ErrorMessage = string.Empty };
public static readonly OperationResult AccessDenied
= new OperationResult() { isSuccess = false, errorCode = 4,
ErrorMessage = "Access is denied." };
public static readonly OperationResult DatabaseNotAvailable
= new OperationResult() { isSuccess = false, errorCode = 17,
ErrorMessage = "Database is not available." };
}
When OperationResult enumeration is replaced with this class, the rest of the code continues working in exactly the same way as before. This is because server was relying on static fields of the enumeration class, while client was relying on methods. We have provided both in the replacement class and, after a rebuild, the output produced by the application is the same as in previous version.
But now we have one addition: OperationResult class exposes a property with error message. So we are free to modify the client class to present error message to the user:
public void ExecuteComplexOperation()
{
Console.WriteLine("Preparing data...");
while (true)
{
Console.WriteLine("Calling server...");
OperationResult status = this.server.DoSomething();
if (status.IsSuccess())
break;
Console.Write("{0}. Do you want to retry? (Y/N) ", status.ErrorMessage);
bool retry = (Console.ReadLine() == "Y");
if (!retry)
break;
}
Console.WriteLine("Done.");
Console.WriteLine(new string('-', 20));
}
And here is the output from the modified application:
Preparing data...
Calling server...
Done.
--------------------
Preparing data...
Calling server...
Access is denied.. Do you want to retry? (Y/N) N
Done.
--------------------
Preparing data...
Calling server...
Database is not available.. Do you want to retry? (Y/N) Y
Calling server...
Done.
--------------------
Preparing data...
Calling server...
Database is not available.. Do you want to retry? (Y/N) Y
Calling server...
Access is denied.. Do you want to retry? (Y/N) N
Done.
--------------------
This output looks much friendlier than before. No cryptic error codes are displayed to the user. But error codes are still available through the GetErrorCode method, in case that they are needed for logging or similar purposes.
In this article we have demonstrated one rather evolutional approach to software design. In many cases we face simple requirements, like returning a success/failure status from a method. But behind the curtains more complex requirements are hiding. We have demonstrated what kind of problems can occur when method is designed to return simple Boolean flag. In fact, it is advisable to return Boolean only from methods which really answer to a yes/no question: IsItRaining, IsSystemRunning, etc.
Any other method, which performs an operation and reports status back, should avoid Boolean result in general. Instead, start with an enumeration with only two values: Success and Failure. But then avoid testing the result against Success value, but rather introduce extension method IsSuccess. That gives the mapping between operation result and True/False status, but at the same time leaves the door open for extension. This step is important in all cases where new requirements can be expected.
There are, off course, cases where it is clear that all possibilities are exhausted (e.g. data direction can only be Read, Write or Bidirectional). In such cases it is acceptable to bind to enumeration values in code. But if there is the slightest chance that usage is going to be extended beyond scope covered by enumeration, make sure to avoid writing code that depends on enumeration.
Once the true requirements are unveiled, those that require complex objects to be returned as operation result, we will be able to replace the enumeration with corresponding class. The rule of thumb is that enumeration should be promoted to class once the object needs to return values that cannot be represented by integer number, which is the underlying value for enumeration. In example above, when string property was requested, that was the signal to promote enumeration to class.
If you wish to learn more, please watch my latest video courses
In this course, you will learn the basic principles of object-oriented programming, and then learn how to apply those principles to construct an operational and correct code using the C# programming language and .NET.
As the course progresses, you will learn such programming concepts as objects, method resolution, polymorphism, object composition, class inheritance, object substitution, etc., but also the basic principles of object-oriented design and even project management, such as abstraction, dependency injection, open-closed principle, tell don't ask principle, the principles of agile software development and many more.
More...
In this course, you will learn how design patterns can be applied to make code better: flexible, short, readable.
You will learn how to decide when and which pattern to apply by formally analyzing the need to flex around specific axis.
More...
This course begins with examination of a realistic application, which is poorly factored and doesn't incorporate design patterns. It is nearly impossible to maintain and develop this application further, due to its poor structure and design.
As demonstration after demonstration will unfold, we will refactor this entire application, fitting many design patterns into place almost without effort. By the end of the course, you will know how code refactoring and design patterns can operate together, and help each other create great design.
More...
In four and a half hours of this course, you will learn how to control design of classes, design of complex algorithms, and how to recognize and implement data structures.
After completing this course, you will know how to develop a large and complex domain model, which you will be able to maintain and extend further. And, not to forget, the model you develop in this way will be correct and free of bugs.
More...
Zoran Horvat is the Principal Consultant at Coding Helmet, speaker and author of 100+ articles, and independent trainer on .NET technology stack. He can often be found speaking at conferences and user groups, promoting object-oriented and functional development style and clean coding practices and techniques that improve longevity of complex business applications.