Skip to content

Instantly share code, notes, and snippets.

@jehugaleahsa
Last active October 22, 2020 18:59
Show Gist options
  • Save jehugaleahsa/3feb30a3208da9efc86a9b031356e592 to your computer and use it in GitHub Desktop.
Save jehugaleahsa/3feb30a3208da9efc86a9b031356e592 to your computer and use it in GitHub Desktop.
Good model design

Good model design

If you implement modern applications, you are probably familiar with the concept of view models and API models. Just to be clear, these are dumb classes normally made up of simple getters and setters (aka., properties) useful for passing data around. When working with REST services, these classes are usually serialized to JSON. You usually don't want them to have a whole lot of functionality since it won't get serialized. Instead, these are denormalized objects that save the UI or other clients from performing business logic.

About 5 years ago, I was working on a project where I had to quickly define a new set of models for the REST API. On a previous project, I had run into a lot of problems with deeply nested models - in other words, models containing other models. On this new project, I let that bad experience influence my decision making. To not nest my models, I flattened everything into one giant, denormalized model. The team immediately ran into issues, though, because we would have two or more sets of addresses in the same model, like a sales contact address and a billing address. When trying to display these addresses in the UI, we couldn't reuse an "address" component by simply passing it an address model. Then on the backend, we couldn't easily copy the address from the model into the internal representation. There were simply too many lost opportunities for code reuse. After that initial failure, we as a team decided to switch back to nested models. I was forced to face the problems I had run into with nested models in the previous project and actually found several patterns that simplified the whole process (and no, I didn't start using Automapper). In retrospect, nesting models is the way to go.

My hope is to quickly share some of those patterns.

Reusable models and mappers

Just like with a database, models should be normalized. This means use nesting as much as possible and keep the properties in each model to a bare minimum. For sake of an example, let's define a couple models in TypeScript:

export interface CustomerModel {
    Id: number;
    Name: string;
    SalesContactAddress: AddressModel;
    BillingAddress: AddressModel;
}

export interface AddressModel {
    Street1: string;
    Street2: string;
    Street3: string;
    City: string;
    State: string;
    Zip: string;
}

Now understand that the structure of these models is specific to one particular, imaginary application; your application might require something different, but let's try to keep this simple. If you were following along, you'd realize my first attempt at defining a "customer" model would have looked something like this:

export interface CustomerModel {
    Id: number;
    Name: string;
    SalesContactStreet1: string;
    SalesContactStreet2: string;
    SalesContactStreet3: string;
    SalesContactCity: string;
    SalesContactState: string;
    SalesContactZip: string;
    BillingStreet1: string;
    BillingStreet2: string;
    BillingStreet3: string;
    BillingCity: string;
    BillingState: string;
    BillingZip: string;
}

On the client side, it would have been very difficult to create an address component accepting a CustomerModel. It wouldn't know whether it should work with sales contact or billing information and those property names probably wouldn't make sense to reuse in other models using addresses anyway. You could probably do some sort of trickery to get this to work, but it's not worth the effort. On the other hand, you can easily create a component accepting an AddressModel and two-way model binding just works.

On the backend, you find yourself in a very similar situation. It should be relatively easy to copy values out of the model into your other classes. The most obvious way to consistently copy values from an object is to provide an interface like IAddress and implement it. But you can't implement an interface in the same class twice! How would your code know when IAddress referred to the sales contact or billing information? Simply put: it couldn't. Again, you could get tricky, but it isn't worth the effort.

In hind-sight, not nesting models simply doesn't scale and... is just a bad idea. With broken out models, it's much easier to define a brainless "mapper" class to convert to and from AddressModel:

public class AddressMapper
{
    public AddressModel GetModel(IAddress address)
    {
        if (address == null)
        {
            return null;
        }
        var model = new AddressModel();
        model.Street1 = address.Street1;
        // ... other fields
        return model;
    }

    public void Update(IAddress address, AddressModel model)
    {
        if (address == null)
        {
            throw new ArgumentNullException(nameof(address));
        }
        if (model == null)
        {
            throw new ArgumentNullException(nameof(model));
        }
        address.Street1 = model.Street1;
        // ... other fields
    }
}

It is tempting, looking at this code, to have AddressModel implement IAddress. You could then implement GetModel like this:

public AddressModel GetModel(IAddress address)
{
    if (address == null)
    {
        return null;
    }
    var model = new AddressModel();
    Update(model, address);
    return model;
}

I would steer clear of this temptation. Consider what would happen if you used a refactoring tool to rename a property in the interface? You'd inadvertantly break your API contract with all your clients (oops!). I feel your backend code should always be free to change, so long as your contracts stay the same. It's more typing but I think it's worth it to keep your client and backend independent.

What a mapper looks like

Those two methods, GetModel and Update, are pretty much the only methods that should ever appear in a mapper. If you are tempted to provide various overloads accepting different parameters, it probably means you are doing something wrong. In fact, you should also notice that mappers are pretty light on dependencies. Strictly speaking, you shouldn't encounter any business logic in your mappers. The only dependencies you should find are other mappers and maybe some basic utilities. For example, here is what the CustomerMapper might look like:

public class CustomerMapper
{
    private readonly AddressMapper addressMapper;

    public CustomerMapper(AddressMapper addressMapper)
    {
        this.addressMapper = addressMapper ?? throw new ArgumentNullException(nameof(addressMapper));
    }

    public CustomerModel GetModel(Customer customer)
    {
        if (customer == null)
        {
            return null;
        }
        var model = new CustomerModel();
        model.Id = customer.Id;
        model.Name = customer.Name;
        model.SalesContactAddress = addressMapper.GetModel(customer.SalesContactAddress);
        model.BillingAddress = addressMapper.GetModel(customer.BillingAddress);
        return model;
    }

    public void Update(Customer customer, CustomerModel model)
    {
        if (customer == null)
        {
            throw new ArgumentNullException(nameof(customer));
        }
        if (model == null)
        {
            throw new ArgumentNullException(nameof(model));
        }
        customer.Name = model.Name;
        addressMapper.Update(customer.SalesContactAddress, model.SalesContactAddress);
        addressMapper.Update(customer.BillingAddress, model.BillingAddress);
    }
}

But what about calculated properties, you might ask? If you need to execute some business logic to populate your models, you should probably not perform it in the mapper itself. Let's say you want to include the customer's annual revenue-to-date in the model (eg., AnnualRevenue). In that case, your mapper really shouldn't be working with a raw Customer object. Instead, you should be working with a business object that knows how to perform that calculation on behalf of the mapper. For example, you might have a simple business object like this:

public interface ICustomerCalculator
{
    Customer Customer { get; }

    decimal CalculateAnnualRevenue();
}

In that case, you would implement your GetModel method accepting the ICustomerCalculator interface rather than a raw Customer. More on that below...

Keep it simple, keep it small

I should point out that I still ran into major problems, even after factoring out my models into smaller, nested models. Those problems are actually related to our AnnualRevenue property above. Did it bother you that we had to lug around this calculated value everywhere? What if we are working on a screen that simply doesn't care about annual revenue? What if a dozen other clients come along needing different calculated values? How often have you needed properties to control what the user can see on the screen (aka., access control)? These sorts of properties add up quickly in large applications.

During my first pass at the REST API, I kept adding on more and more properties with each new screen/component I wrote. I had so many at one point that I started passing along an "options" object that looked like this:

public class CustomerOptions
{
    public bool IncludeAnnualRevenue { get; set; }
    public bool IncludeOrderCount { get; set; }
    public bool IncludeOrderHistory { get; set; }
    public bool IncludeCreditCardInformation { get; set; }
    // ... and so on
}

My mapper would then inspect these options to figure out what to calculate. My mapper was full of nasty if-statements and I had to make properties like AnnualRevenue nullable (aka. decimal?) to distiguish between "not set" and zero.

There's a simple solution that avoids this mess: create a separate model for each client/screen that needs something different. For example, I might create a separate model for the customer called CustomerSalesModel:

export interface CustomerSalesModel {
    Customer: CustomerModel;
    AnnualRevenue: number;
}

I would then create a separate mapper, too:

public class CustomerSalesMapper
{
    private readonly CustomerMapper customerMapper;

    public CustomerSalesMapper(CustomerMapper customerMapper)
    {
        this.customerMapper = customerMapper ?? throw new ArgumentNullException(nameof(customerMapper));
    }

    public CustomerSalesModel GetModel(ICustomerCalculator calculator)
    {
        if (calculator == null)
        {
            return null;
        }
        var model = new CustomerSalesModel();
        model.Customer = customerMapper.GetModel(calculator.Customer);
        model.AnnualRevenue = calculator.CalculateAnnualRevenue();
        return model;
    }
}

Notice a few things: 1) the customer mapper goes back to taking a simple Customer and 2) the CustomerSalesMapper doesn't provide an Update method. If you are worried about code getting duplicated for each model, keep in mind that the logic for mapping a customer model stays in the customer mapper and the logic for performing calculations stays in the ICustomerCalculator. The only potential duplication, really, is passing in the CustomerMapper as a dependency.

It takes a lot of vigilance and effort to keep your models minimal and create the appropriate wrapper models. However, it saves from performing a lot of unnecessary work and from your models growing more and more as the application matures. This also makes it much easier to compose models, since you don't need to lug around a lot of unnecessary baggage.

Conclusion

Like I said, I wanted to keep this short and sweet. I hope that I provided a decent illustration of the techniques behind designing a reusable, long-term model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment