DRY-ing code with Command Handler

Keeping code DRY is a challenge for any developers. When we were studying programming, we were measured and taught that duplication is bad. In some cases, our score were measured by the number of code duplication. Working in the industry, we heard the idea of “It’s ok to duplicate”. We were happy with this idea for a while, and so in our code now, we have duplication and accepted it as part of programming whilst we convince ourselves that we will re-factor this when it grows. Let’s face it, some of the codes that we created will probably never be re-factored. They’ve grown too much, and we only wrote tests for some but not all.

Now, let me get this straight. I don’t have a problem with duplication. I have a problem with duplication that happened because we are just too lazy to do the right way. I like to think that duplication is a ‘flag’ that says “Please keep attention of me”. When you only have 1 place that the code duplicated, it is still maintainable, and easy to look for.But when we start having to copy and paste the code to more places, and when we want to change a thing becomes a treasure hunt, this is means, there are abstraction(s) missing that supposedly to happen.

Let’s consider the following snippet:

BookAppointmentInputDto.cs

public class BookAppointmentInputDto
    {
        [Required]
        public int PatientId { get; set; }

        [Required]
        public int RoomId { get; set; }

        [Required]
        public int ScheduleId { get; set; }
    }

AppointmentService.cs

public ServiceResponse BookAppointment(BookAppointmentInputDto input)
        {
            var isRoomFree  = _scheduleRepository.IsRoomScheduleFree(input.roomId, input.scheduleId);
            if (!isRoomFree)
                return ServiceResponse.Failed("Room is not free. Please select other time.");

            var isPatientFree = _scheduleRepository.isPatientFree(input.scheduleId, input.patientId);
            if (!isPatientFree)
                return ServiceResponse.Failed("Patient already have an appointment at this time. Please book other time.");

            // Process appointment booking
            try {
                var newAppointment = new Appointment();
                newAppointment.PatientId = input.PatientId;
                newAppointment.RoomId = input.RoomId;
                newAppointment.ScheduleId = input.ScheduleId;

                // Persist into the db
                var newAppointmentId = appointmentRepository.Insert(newAppointment);
                _context.SaveChanges();

                // Log the action.
                Logger.Log(“Appointment booked”);

                // return
                return ServiceResponse.Success(newAppointmentId);
            }
            catch(Exception e) {
                return ServiceResponse.Failed(e);
            }
        }

The above code is an example of a typical service class with exceptions catching and several validations. Whilst they are perfectly valid and ok, the code will become a maintenance nightmare when the file grew.

These are few of code duplication that we can see from the code above:

  1. Validation Rules
  2. Error/Exceptions Handling
  3. Logging

Imagine adding few more methods to the above such as “RescheduleAppointment”, “CancelAppointment” and “PatientFailedToAttendAppointment” into the service class. Multiply this with 3 other service classes that you will be adding within the next week.

Any developer coding the service layer must inherently think about how they would like to tackle the code smell. Trying to keep them DRY becomes a chore. This usually results in an inconsistent code state, where some part may have logging or error/exceptions handling, and some aren’t. Some part may have their own error and exceptions handling, and making it even more confusing to maintain. This means we cannot ensure quality and consistency throughout the code.

What we want instead is to focus on values. The values in the function above lies in the validations that happen prior processing the request, and processing the request itself.

Compare to the following snippet:

BookAppointmentCommand.cs

public class BookAppointmentCommand : ICommand {

        [Required]
        public int PatientId { get; set; }
        [Required]
        public int RoomId { get; set; }
        [Required]
        public int ScheduleId { get; set; }

        public class CreateAppointmentMapper : ICreateCommandMapper<BookAppointmentCommand, Appointment> {
            public Appointment Create(BookAppointmentCommand command)
            {
                var newAppointment = Appointment.Create(command.PatientId, command.RoomId, command.ScheduleId);
                return newAppointment;
            }
        }

        public class RoomMustBeAvailable : ICommandBusinessRuleValidation<BookAppointmentCommand, Appointment>
        {
            private readonly IScheduleRepository _scheduleRepository;

            public RoomMustBeAvailable(IScheduleRepository scheduleRepository) {
                _scheduleRepository = scheduleRepository;
            }

            public ValidationResult Validate(BookAppointmentCommand command) {
                var isRoomFree = _scheduleRepository.IsRoomScheduleFree(input.roomId, input.scheduleId);
                if (!isRoomFree)
                    return ValidationResult.Failed("Room is not free. Please select other time.");
                return ValidationResult.Success();
            }
        }

        public class PatientMustBeAvailable: ICommandBusinessRuleValidation<BookAppointmentCommand, Appointment>
        {
            private readonly IScheduleRepository _scheduleRepository;

            public PatientMustBeAvailable( … ) { … }

            public ValidationResult Validate(BookAppointmentCommand command) {
                var isPatientFree = _scheduleRepository.isPatientFree(input.roomId, input.patientId);
                if (!isPatientFree)
                    return ValidationResult.Failed("Patient already have an appointment at this time. Please book other time.");
                return ValidationResult.Success();
            }
        }
    }

And the service method becomes:

AppointmentService.cs

public ServiceResponse BookAppointment(BookAppointmentCommand command) {
    var handler = _handlerFactory(command);
    return handler.Handle(command);
}

The alternative snippet with command handler, focuses on the BookAppointmentCommand.cs. All values that we really need out the method BookAppointment were all inside BookAppointmentCommand.cs. All other details about logging and exceptions handled elsewhere (usually with decorator with the handler). The service class becomes a true facade class. We reap the benefits more when we have multiple places that requires exposure of different services.

There are downside to this pattern as well. Designing a command can be a challenge in itself, as ideally we would like a command to not be doing too much. The pattern introduce many classes into the system, and for developers that aren’t used to this yet, it will become a nightmare for them as opposed to the first straightforward snippet example. 

I found that command handler benefits usually outweigh its costs in modern web application context especially if we use this in concurrent with CQRS pattern. I prefer a fast and quick development turn-around, and throwaway code (if you don’t need the command, delete it, if you don’t need the validation, delete it) than a chunky code at any time alongside with logging and exceptions being handled properly.

Related articles:

Do you agree with the article? Do you use command handler pattern?
What other code smells / duplication that you usually see, and how do tackle the problem?

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s