Code Duplication is a Myth

I know that code duplication is not good. We try to avoid code duplication. In a perfect world, you do not have code duplication. What annoys me is that we seem to forget to ask some critical questions to ourselves.

  1. Is it really that bad in all circumstances?
  2. What might be worst if we try to avoid code duplication?

Let’s take a typical web application where you have features to add and edit a user.

With 2 separated models

    public class CreateUserModel
    {
        public string Name { get; set; }
        public string Address { get; set; }
        public DateTime? DateOfBirth { get; set; }
    }

    public class EditUserModel
    {
        public int Id { get; set; }
        public string Name { get; set; }
        public string Address { get; set; }
        public DateTime? DateOfBirth { get; set; }
    }

Or one single User Model?

    public class UserModel
    {
        public int Id { get; set; }
        public string Name { get; set; }
        public string Address { get; set; }
        public DateTime? DateOfBirth { get; set; }
    }

In single user model, if the Id is 0, assume that is creating a new user.

In the real world application, the model is complex. I make this example to demonstrate the point: There are many duplications in 2 models. The question is whether it is good or bad?

Well, it depends … on the context.

Why do we see it duplicated? Mostly because the code looks the same. I, however, want to look at another angle, from the purpose and context angle.

In term of business view, create and edit a user are 2 different features. They operate on the same data object (User). But they are not the same. Let’s assume that we have a web application. There are 2 screens, one for Create and one for Edit.

        public IActionResult CreateUser(CreateUserModel model)
        {
            return View();
        }

        public IActionResult EditUser(EditUserModel model)
        {
            return View();
        }

        // Reuse the same model for 2 different features: Create or Edit
        public IActionResult SaveUser(UserModel model)
        {
            return View();
        }

In the SaveUser action, if the User.Id is 0, use the Create logic, otherwise, use the Edit logic. So we can put aside the logic.

Initially, both Create and Edit screens look the same. They operate on the same piece of data. The single model sounds a right choice. However, I will choose the 2 models. I do not see them duplicated. I do not see benefits of unifying them.

Here are why

  1. Each is an independent feature.
  2. They have different input and output. Sooner or later you will end up some information on a feature not make sense on another.
  3. By separating them, we can have 2 teams (groups) work on 2 features without stepping on each other toes.

Ok then, when will it make sense?

I will consider a duplication when

  1. A piece of code that carries logic. Not just data or POCO.
  2. Independent of context. The refactor code (as a result of killing duplication) might depend on the input parameters. However, keep an eye on the number of parameters and make your instinct decisions base on your experiences.
  3. Easy to understand. There should be a very limit of condition/switching in the implementation.

From my experience, killing duplication is not an easy task. It requires a lot of skills

  1. Skill to ask the right question about the value of the code, the logic of the code, … One should ask themselves serious questions.
  2. Skill to read the code and get a brief of what it does.
  3. Skill to capture just enough to refactor out.
  4. Skill to refactor.

Did I tell you that Naming is one of the hardest tasks? Given that you have a piece of duplicated code, what would you name them?

I fell in love with the slice (with Feature approach) architecture when I first saw it. I saw its value when traditional architecture caused my headache when the code was out of control. Especially, when I have to develop a new feature. The biggest fear when adding new features is the chance of breaking existing features, aka causing regression bugs. With the feature approach, we can limit the risk.

Every approach has its pros and cons. We, developers, have to weigh them and pick the one that fits most.

There are some valuable links from experts.

Jimmy Bogard SOLID Architecture.

Steve Smith ASP.NET Core Feature Architecture.

Application structure: Concepts and Features.

Isolated Feature from Ayende.

Or checkout Jimmy repo here. A wonderful resource to learn how to build a web application with the Slice + MediatR + Feature.

Infrastructure and Application Code – Keep Loose Dependencies

When talking about dependency, many will think of Dependency Injection, think of Interface, think of Container, … Things go like if you have a Controller, that controller consumes a Repository, then you should inject them with dependency injection, most of the time via the constructor.

And we just stop at that level, at that thinking. Not so many ask for how to detect dependencies. Why do we have to think so much when things just work? No, we do not. However, it would be better if we dig deeper and understand more.

To identify dependencies, we have to define boundaries. Boundaries are the hardest thing to find, even for experts. There is no silver bullet. But there are some specific areas that we can identify and make a good decision.

Infrastructure and Application are 2 boundaries that I made up. They are too general, to be honest. Let’s start exploring with examples.

Application Settings

From my previous post, I have created an appsettings.json with .NET Core

{
  "AppSettings": {
    "Message": "Setting from app settings"
  }
}

And I have consumer code in AppSettingsController

    public class AppSettingsController
    {
        private readonly IConfigurationRoot _configuration;
        private readonly AppSettings _appSettings;
        private readonly GlobalSettings _globalSettings;
        public AppSettingsController(
            IOptions<AppSettings> appSettingsOptions, 
            IOptions<GlobalSettings> globalSettingsOptions,
            IConfigurationRoot configuration)
        {
            _configuration = configuration;
            _appSettings = appSettingsOptions.Value;
            _globalSettings = globalSettingsOptions.Value;
        }

        public Task GetAppSettings(HttpContext context)
        {
            return context.Response.WriteAsync($"Thie is AppSettingsController: {_appSettings.Message}; " +
                                               $"EF Connection String: {_globalSettings.ConnectionStrings.Ef}; " +
                                               $"Author: {_globalSettings.Author.Name}");
        }

        public Task GetDirectAppSettings(HttpContext context)
        {
            return context.Response.WriteAsync($"Direct app setting: {_configuration["AppSettings:Message"]}");
        }
    }

On the GetDirectAppSettings method, it accesses the setting from _configuration[“AppSetings:Message”]. It works. And look like we have Dependency Injection with IConfigurationRoot.

Then what are problems? What are differences between GetAppSettings and GetDirectAppSetings methods in term of accessing configuration values?

Problem

IConfigurationRoot, even an Interface, is Infrastructure Code, Infrastructure Concern. Whereas Controller is Application Code.

Let’s ask these questions:

  1. Does the AppSettingsController need to know about the present of IConfigurationRoot?
  2. Does it need to know about how to access a configuration value (such as AppSettings->Message)?

Asking questions is a good way to find out dependencies.

Asking questions is a good way to find out dependencies, to define boundaries.

I do not find “Yes” for those questions. Ok then. What does it need? It needs application settings values. It only needs values. And we accidentally tell it how to get them.

If not working with ASP.NET Core, we have ConfigurationManager class. Same problem! If you use it in your application code, you create a dependency.

Solution

In the ASP.NET Core example, the perfect way is reading configuration values at Startup

    public class Startup
    {
        private readonly IConfigurationRoot _configuration;
        public Startup(IHostingEnvironment env)
        {
            var builder = new ConfigurationBuilder()
                .SetBasePath(env.ContentRootPath)
                .AddJsonFile("appsettings.json")
                .AddJsonFile("appComplexSettings.json");

            _configuration = builder.Build();
        }
        public void ConfigureServices(IServiceCollection services)
        {
            // Register the ability to read options in configuration
            services.AddOptions();
            services.Configure<AppSettings>(_configuration.GetSection("AppSettings"));
            services.Configure<GlobalSettings>(_configuration);
        }
    }

And controllers consume values via IOptions. Look at the AppSettingsController->GetAppSettings method.

A valid concern is that IOptions is also an Infrastructure code. In service code, we might not want to depend on IOptions to get the values. We might want to inject AppSettings directly. That should be a fairly easy task.

Impact

In a web application, for example, built with ASP.NET MVC, Controllers can access/accept that dependency with less impact. Because in fact, MVC itself is Infrastructure.

However, when it comes to service, domain, repository code, especially reusable components, it has a huge impact. Why? Because obviously, it depends on the infrastructure, the IConfigurationRoot for example. You cannot take those components and use other places where IConfigurationRoot does not exist.

It also creates another impact on developers mindset. Developers should develop a correct mindset while coding. They might have to think of dependencies, of boundaries, of portability. It is hard to get things right. But it is not hard to start thinking now.

Recap

Infrastructure and Application code is an example of identifying code boundaries. Asking questions is a good way to find them. I do not suggest you ask questions for any piece of code you write. I encourage you to start now. Create a habit of asking questions about your code. Start with methods, classes, then move on with Assembly level.

Try to look at things in boundaries, dependencies, instead of a big ball of mud.

Refactor a 6K Line of Code Domain Class

I have been working on a legacy project. It is a complex and big system. Throughout the codebase, there is a big domain object class.

How big is it, man?

Not much dude. It is just 6k (6.000) line of C# code.

One day, I found out a way to improve it. So the story begins.

Once Upon a Time

Imagine that class is School. School will have a collection of Students, Teachers, Labors, Rooms, Chairs, … The School class is a root domain. Therefore, they decided that all the methods must go through it.

There are typical patterns related to a collection: Add, Edit, Remove. Such as Add Student, Update Student, Remove Student, ..

It is a typical pattern we have seen many places. Except, that class became a ghost with 6.000 (6K) Line of Code. There are so many problems with such a codebase. But it is out of the scope of this post.

Pre-conditions

  1. Legacy code written with .NETt 4.0 (and still must be in 4.0).
  2. NHibernate as ORM framework.
  3. Look like DDD with CQRS (even none of them are actually true).
    public class School
    {
        private readonly IList<Student> _students;
        private readonly IList<Teacher> _teachers;
        private readonly IList<Room> _rooms;
        private readonly IList<Chair> _chairs;

        public virtual void AddStudent(Student student)
        {

        }

        public virtual Student FindStudent(Guid id)
        {
            return new Student();
        }

        public virtual void Update(Student student)
        {

        }

        public virtual void Remove(Student student)
        {

        }
    }

    public class Student
    {
        
    }

    public class Teacher
    {
        
    }

    public class Room
    {
        
    }

    public class Chair
    {
        
    }

There are a couple of things to remember first

  1. It is a legacy codebase. Which mean we did not design it in the first place. We took it over.
  2. Time is limited. Documentation is a luxury to have. In short, we cannot just rewrite or redesign it.
  3. It is painful.

How to refactor?

The goals should be:

  1. Reduce the number of line of code. Kill the ghost.
  2. Keep the visibility protection at the domain level. In DDD, the setters are protected or private.
  3. Improve responsibilities management.

Sometimes, Visual Studio has problems with opening the class 🙁 Poor it!

Superstar Needs Managers

The idea is quite simple. I got the inspiration from Superstar Metaphor. A superstar has many things: Properties, Cash, Connection, Style, Contracts, … Everything is attached to them. However, there is no way they can manage by themselves. They need managers. They need a manager for each aspect.

  • Property Manager: Manage properties
  • Financial Manager: Manage finance
  • Style Manager: Manage style
  • And so on.

Apply Composite Pattern

With that Superstar Metaphor, we can apply on School class. It is quite easy with some simple steps

Identify Areas

First, we have to identify areas that need outsource to managers. This step depends on your project context.

In our silly example, they are Student, Teacher, Room, and Chair.

Hide Managers

In Visual Studio (code) it is simply creating new classes 😛

We have StudentManager, TeacherManager, RoomManager, and ChairManager.

Delegate Responsibilities to Managers

Move the logic to proper managers and keep the old interface for School.

Show Me the Code

Yes, Sir!

    public class School
    {
        protected internal readonly IList<Student> _students = new List<Student>();
        private readonly IList<Teacher> _teachers = new List<Teacher>();
        private readonly IList<Room> _rooms = new List<Room>();
        private readonly IList<Chair> _chairs = new List<Chair>();

        private readonly StudentManager _studentManager;

        public School()
        {
            _studentManager = new StudentManager(this);
        }

        public virtual void AddStudent(Student student)
        {
            _studentManager.Add(student);
        }

        public virtual Student FindStudent(Guid id)
        {
            return new Student();
        }

        public virtual void Update(Student student)
        {

        }

        public virtual void Remove(Student student)
        {

        }
    }

    public class StudentManager
    {
        private readonly School _school;

        public StudentManager(School school)
        {
            _school = school;
        }

        public void Add(Student student)
        {
            _school._students.Add(student);
        }
    }

I applied the manager approach for Student area. The same goes for other areas: Teacher, Room, and Chair.

Protected Internal Modifier

Instead of using private or protected, we use protected internal modifier. With this modifier, we protect the domain from outside assembly. That is we just moved from class level to assembly level.

This modifier level allows managers to access Superstar’s properties.

Hey Cool Man!

Yeah. Even I said so to myself. Some of the immediate result I got after 2-3 hours

  1. Reduce more than 1K LoC in the Ghost class.
  2. Create managers to handle correct responsibilities.
  3. Feel a better control of the codebase
  4. Stop the trend of adding more code into the Ghost.

The best part is that, as a developer, I feel rocked 🙂 (yes, till now)__the feeling of getting a cool thing done.

Design and implement a template engine–part 2

In the Design and implement a template engine – part 1 I mentioned about a better solution: Reuse Razor Engine. In the post, I go in detail of how to implement it. But, first, we need to know some concepts:

Concepts

  1. Template Engine: Use to render a template into string
  2. Template Model Builder: Build the Model object which is used to fill in the data in template
  3. Template Model Expander: Expand the Model object with more properties
  4. And Template: the template

Implementation

The implementation assume that you already know about dynamic object and Razor Syntax (which is used in MVC – Views)

public interface ITemplateModelExpander
{
    /// <summary>
    /// Expand a dynamic object with more properties. The input object is sharing between expanders. Each expander will expand the properties that most make sense to them
    /// </summary>
    /// <param name="obj"></param>
    /// <returns></returns>
    dynamic Expand(dynamic obj);
    /// <summary>
    /// True if want to tell the <see cref="ITemplateModelBuilder"/> that, use the result of Expand method as the input for the next expander.
    /// The implementation should be very careful when setting true. Most of the time, it should be false
    /// </summary>
    bool UseResultForNextExpander { get; }
}

ITemplateModelExpander is the one that will be implemented in many classes. As its purpose, we want to expand the dynamic object to build up a real Model for the template.

Next we will see how we define a Template Model Builder.

/// <summary>
/// Builder is for building the object model using the inner expanders
/// </summary>
public interface ITemplateModelBuilder
{
    /// <summary>
    /// return the dynamic object that can be used in template
    /// </summary>
    /// <returns></returns>
    object Build();
    /// <summary>
    /// Append expander
    /// </summary>
    /// <param name="expander"></param>
    void AppendExpander(ITemplateModelExpander expander);
    void AppendExpanders(IEnumerable<ITemplateModelExpander> expanders);
    /// <summary>
    /// Clear all expanders. It is useful when you want to empty the object model and start adding new expander.
    /// Mostly you will use it if there is a single <see cref="ITemplateModelBuilder"/> in the system (Singleton lifestyle)
    /// </summary>
    void ClearExpanders();
}

It does 1 main thing: Build; which builds the Model object (of course, dynamic object) using expanders. That’s why you see 3 more methods related to Expander.

Then I make a simple implementation for the Model Builder:

public class StandardTemplateModelBuilder : ITemplateModelBuilder
{
    private readonly List<ITemplateModelExpander> _expanders = new List<ITemplateModelExpander>();

    public object Build()
    {
        var model = new ExpandoObject();
        foreach (var expander in _expanders)
        {
            var result = expander.Expand(model);
            if (expander.UseResultForNextExpander)
            {
                model = result;
            }
        }
        return model;
    }

    public void AppendExpander(ITemplateModelExpander expander)
    {
        _expanders.Add(expander);
    }

    public void AppendExpanders(IEnumerable<ITemplateModelExpander> expanders)
    {
        _expanders.AddRange(expanders);
    }

    public void ClearExpanders()
    {
        _expanders.Clear();
    }
}

Pretty simple right Smile

Let’s talk about Template, I have this simple interface:

/// <summary>
/// Define a template entry.
/// </summary>
public interface ITemplate
{
    /// <summary>
    /// The template, which tell <see cref="ITemplateEngine"/> how to render output
    /// </summary>
    string Template { get; set; }
    /// <summary>
    /// The builder which will build the model to populate data for the template
    /// </summary>
    ITemplateModelBuilder ModelBuilder { get; }
}

Define it as interface gives me power to change the template type. Even though, I just have Razor template for now:

public class RazorTemplate : ITemplate
{
    public RazorTemplate(ITemplateModelBuilder modelBuilder)
    {
        if(modelBuilder == null)
            throw new ArgumentNullException("modelBuilder");
        ModelBuilder = modelBuilder;
    }
    public string Template { get; set; }
    public ITemplateModelBuilder ModelBuilder { get; private set; }
}

You might wander why do I not inject template property directly in the constructor? Because, I want to resolve the ITemplate instance using an IoC container, such as Windsor and wire up with correct ITemplateModelBuilder which is StandardTemplateModelBuilder in this case. (I will show you how to use it later)

The last part, the Template Engine:

/// <summary>
/// Engine to render output base on template
/// </summary>
public interface ITemplateEngine
{
    string Render(ITemplate template);
}

Very simple. It just render an ITemplate instance.

The implementation is based on Razor Engine

/// <summary>
/// Simple implementation using Razor engine
/// </summary>
public class RazorBasedTemplateEngine : ITemplateEngine
{
    public string Render(ITemplate template)
    {
        return Razor.Parse(template.Template, template.ModelBuilder.Build());
    }
}

It asks the ModelBuilder from Template to build up the dynamic model object which is used to populate the template.

Usage

Have a look at this Unit Test:

[Test]
public void Test_simple_render_engine()
{
    // Arrange
    var engine = new RazorBasedTemplateEngine();
    var modelBuilder = new StandardTemplateModelBuilder();
    var template = new RazorTemplate(modelBuilder)
    {
        Template = @"Hello @Model.Name"
    };
    modelBuilder.AppendExpander(new SampleByNameExpander("Thai Anh Duc"));
    // Act
    var result = engine.Render(template);
    // Assert
    Console.WriteLine(result);
    Assert.AreEqual("Hello Thai Anh Duc", result);
}

I setup Engine, ModelBuilder, and Razor Template. These objects can be wire up automatically by IoC, see below

public class TestIoCWireUp
{
    private readonly ITemplateEngine _engine;
    private readonly ITemplate _template;

    public TestIoCWireUp(ITemplateEngine engine, ITemplate template)
    {
        if (engine == null)
            throw new ArgumentNullException("engine");
        if (template == null)
            throw new ArgumentNullException("template");
        _engine = engine;
        _template = template;
    }

    public string RenderNameTemplate(string template)
    {
        _template.Template = template;
        _template.ModelBuilder.AppendExpander(new SampleByNameExpander("Thai Anh Duc"));
        return _engine.Render(_template);
    }
}

And very simple implementation for SampleByNameExpander:

public class SampleByNameExpander : ITemplateModelExpander
{
    private readonly string _name;

    public SampleByNameExpander(string name)
    {
        _name = name;
    }
    public dynamic Expand(dynamic obj)
    {
        obj.Name = _name;
        return obj;
    }

    public bool UseResultForNextExpander { get {return false;} }
}

If you want to expand the Model with more properties that make sense to your application, you can add as many as possible. Just make sure you add the expander for the ModelBuilder of the Template you want. Or you can create a big giant Expander which will expose lot of properties at once. However, I will not encourage that. It is much more maintainable with simple small classes.

And, that’s it. Hope it help Smile

Design and implement a template engine – part 1

Recently, I got a task which is typical in many systems: Template with merge fields. The common usage is for email.

Given that an employee is created, the system will notify the manager by email with template:

Dear Manager,

New employee {Name} has been created on {DateTime.Now}.

From the above statement, we have template with 2 merged fields: Employee Name and Created Date. There are some solutions. But I name the old way of doing it here for reference:

Replace merge fields using string.Replace or Regex.

You can imagine it and how to do it. Or you event did it some times.

A better approach: Take advantages of Razor Engine.

The template will be in razor syntax

Dear Manager,

New employee @Model.EmployeeName has been created on @DateTime.Now.ToString(“s”).

In the next post, I will discuss about implementation.