Are your class constructors heavy?

One of OOP principles says, that every class should enter valid states only. The bigger class it is, the harder to do. Typically we use class constructors so put class into that valid state. But it’s uncomfortable to have dozen of parameters as it becomes disorganized and difficult to use. Let’s say, your class is persisted to database, so typically has 2 constructors. First for a new instance and second one for the existing instance read from the database (and in that state). Hierarchical class structure adds even more complexity. How to simplify?

Example

Consider following example. We’re building a message queue and so it contains processing records. It can do anything, imagine it’s a job which is triggered at some point and results are written back to the database. The record has a message serialized in json, which tells the handler what to do, and then there is some message processor (handler of the message) responsible to deal with that. Retries counter is there to know how many times the processing failed in total or daily, and after xx fails it disables the job and notifies someone. The states are at least New, Running, Finished, Failed, etc..

On the SQL side:

CREATE TABLE [dbo].[Processing](
    [Id] [uniqueidentifier] NOT NULL,	
    [ProcessingId] [int] IDENTITY(1,1) NOT NULL,	
    [Message] [ntext] NOT NULL,	
    [ProcessingTypeId] [int] NOT NULL,	
    [ProcessingSubTypeId] [int] NULL,	
    [IsProcessing] [bit] NOT NULL,	
    [Retries] [int] NOT NULL,	
    [RetryCounter] [int] NOT NULL,	
    [IsEnabled] [bit] NOT NULL,	
    [StartDate] [datetime] NULL,	
    [FinishDate] [datetime] NULL,	
    [ProcessedDate] [datetime] NULL,	
    [ProcessingResult] [ntext] NULL)

… and domain object:

class ProcessingMessage
{ 
    public ProcessingMessage(string processingType, string processingSubType, string message) 
    {  
        Id = Guid.NewGuid();    
        ProcessingType = processingType;   
        ProcessingSubType = processingSubType;   
        Message = message; 
    } 
    
    public ProcessingMessage(Guid id, DateTime? dateStarted, DateTime? dateFinished, DateTime? dateProcessed, string processingType, string processingSubType, string message, int retryCounter, int maxRetries, int retries, bool isEnabled, ProcessingResult result, bool isProcessing) 
    {   
        Id = id;   
        DateStarted = dateStarted;   
        DateFinished = dateFinished;   
        DateProcessed = dateProcessed;   
        ProcessingType = processingType;   
        ProcessingSubType = processingSubType;   
        Message = message;   
        RetryCounter = retryCounter;   
        MaxRetries = maxRetries;   
        Retries = retries;   
        IsEnabled = isEnabled;   
        Result = result;   
        IsProcessing = isProcessing; 
    } 
    // ...
}

The class has 2 constructors. Whilst the first constructor is not an issue and easy to use, the other is quite heavy. One solution is to break the class into several states, like ProcessingMessageRunning, ProcessingMessageFailed, etc. and have only relevant properties on each. State pattern. However, here it’s not worth the effort. Much simpler is Builder pattern. The idea is to leave the responsibility to correctly instantiate the class to someone else, who knows how. I always tend to have my Builder nested class so I can set private fields directly. Why not public methods or properties? Methods may have additional logic to keep your class in a valid state. Property setters are not public visible - almost never - again, don’t let anyone to change state of your class from outside directly.

Fluent interface then adds better readability to the code.

public class ProcessingMessage 
{ 
    private ProcessingMessage(Guid id, string processingType, string message, bool isProcessing, bool isEnabled)  
    {   
        Id = id;    
        ProcessingType = processingType;    
        Message = message;    
        IsProcessing = isProcessing;    
        IsEnabled = isEnabled;  
    }  
    
    public class Builder  
    {    
        private readonly ProcessingMessage message;    
        private Builder(ProcessingMessage message)    
        {     
            this.message = message;    
        }    
        
        public static Builder New(string processingType, string msg)    
        {      
            var pm = new ProcessingMessage(Guid.NewGuid(), processingType, msg, false, true)      
            {        
                IsEnabled = true      
            };      
            return new Builder(pm);    
        }    
        
        public static Builder Existing(Guid id, string processingType, string message, bool isProcessing, bool isEnabled)    
        {      
            var pm = new ProcessingMessage(id, processingType, message, isProcessing, isEnabled);      
            return new Builder(pm);    
        }    
        
        public Builder WithSubType(string processingSubType)    
        {      
            message.ProcessingSubType = processingSubType;      
            return this;    
        }
        
        public Builder StartedOn(DateTime? dateStarted)    
        {      
            message.DateStarted = dateStarted;      
            return this;    
        }    
        
        public Builder TimesRetried(int totalRetries, int dailyRetries)    
        {      
            message.Retries = totalRetries;      
            message.RetryCounter = dailyRetries;      
            return this;    
        }    
        
        public Builder FinishedOn(DateTime? dateFinished)    
        {      
            message.DateFinished = dateFinished;      
            return this;    
        }    
        
        public Builder WithResult(ProcessingResult processingResult)    
        {      
            message.Result = processingResult;      
            return this;    
        }    
        
        public Builder WithResult(string jsonResult)    
        {      
            if (string.IsNullOrWhiteSpace(jsonResult))        
                return this;      
                
            message.Result = ProcessingResult.FromJson(jsonResult);
            return this;
        }    
        
        public ProcessingMessage Build()    
        {      
            return message;    
        }  
    }
}

Then it’s ProcessingRepository’s responsibility to retrieve data from database and create my domain object.

ProcessingMessage.Builder
    .Existing(entity.Id, processingType, entity.Message, entity.IsProcessing, entity.IsEnabled)
    .StartedOn(entity.StartDate)
    .TimesRetried(entity.Retries, entity.RetryCounter)
    .FinishedOn(entity.FinishDate)
    .WithResult(entity.ProcessingResult)
    .Build();

Again, important rule to follow is that your class should enter valid states only. Looking at the previous example we could say, when I call FinishedOn it should also have ProcessingResult as a parameter, because when the call is finished we must have some result.

2022

Greetings to Camunda, Zeebe and Dapr!

2 minute read

It’s awesome to see how Dapr and Camunda workflow engine work well together. By using BPMN and DMN we completely remove business logic from the code and only...

Back to Top ↑

2021

Are your class constructors heavy?

3 minute read

One of OOP principles says, that every class should enter valid states only. The bigger class it is, the harder to do. Typically we use class constructors so...

Back to Top ↑

2020

Back to Top ↑