Search code examples
c#design-patternsrefactoring

How to refactor this code so I can make changes more easily


I have around 40 datasets forms on my project split in 8 dataset groups.I ll try to explain as simple as possible. Let take IDCards group for example. There I have 2 datasets, ValidIDCardsand ValidIDCardsForeigners. Currently, for every dataset, my backend folder structure looks like (I ll use ValidIDCards as example):

ValidIdCards
    -Commands
        -Create
              - CreateValidIDCardsCommandModel
              - CreateValidIDCardsCommandHandler
              - CreateValidIDCardsCommandValidator
        -Update
              - CreateValidIDCardsCommandModel
              - CreateValidIDCardsCommandHandler
              - CreateValidIDCardsCommandValidator

        -Other commands..

    -Queries
            -GetAll
                  - GetAllValidIDCardsQueryModel
                  - GetAllValidIDCardsQueryHandler
                  - GetAllValidIDCardsQueryViewModel
            -GetbyId
                  - GetByIdValidIDCardsQueryModel
                  - GetByIdValidIDCardsQueryHandler
                  - GetByValidIDCardsQueryViewModel

           -Other queries..

Now, take a look at Create command. CreateValidIDCardsCommandModel looks like:

 public class CreateValidIDCardsCommandModel : IRequest<int>
{
    public int Id { get; set; }
    public string Institution { get; set; }
    public int Total { get; set; }
    public int CountryID{ get; set;} //foreign key to another table
}

CreateValidIDCardsCommandHandler looks like:

 public async Task<int> Handle(CreateValidIDCardsCommandModel request, CancellationToken cancellationToken)
    {       
      var country = _context.Countries.Where(x => 
      x.Code == request.CountryID)).FirstOrDefault();

        var newItem = new Domain.Entities.ValidIdcard
        {
            Institution = request.Institution,
            Total = request.Total,
            CountryID = country.number,
        };
        _context.ValidIdcards.Add(newItem);     
        await _context.SaveChangesAsync();
    }

and in CreateValidIDCardsCommandValidator I validate those fields.

Now take a look at Create command in ValidIDCardsForeigners. Model is:

     public class CreateValidIDCardsForeignerCommandModel : IRequest<int>
{
    public int Id { get; set; }
    public int Total { get; set; }
    public string Citizenship {get; set;}
    public int CountryID{ get; set;} //foreign key to another table
}

Handler looks like:

 public async Task<int> Handle(CreateValidIDCardsForeignersCommandModel request, CancellationToken cancellationToken)
{
      var country = _context.Countries.Where(x => 
      x.Code == request.CountryID)).FirstOrDefault();

    var newItem = new Domain.Entities.ValidIdcardForeigner
    {
        Total = request.Total,
        CountryID = country.number,
        Citizenship = request.Citizenship;
    };
    _context.ValidIdcards.Add(newItem);     
    await _context.SaveChangesAsync();
}

As you can see, difference between ValidIDCards and ValidIDCardsForeigners is in two fields in Model and different new Domain.Entities.Entity in Handler Now, problem is if I for example want to change field CountryID to string(and this is real life situation that happened to me) . I need to edit 40 datasets, several commands and queries for every dataset, (and that is over 100 files for single common change). Also, if I want to add new dataset, it takes me almost a day to copy and change almost 80% of same code.

I thought about making one global CreateCommandModel where I would put all fields that most of datasets have (id, countryID, institution), and then inherit in every dataset, but that would not work because not all of datasets have these fields, and few datasets have totaly different fields. I was looking for help in some design patterns, but could not find one to help me.

So, question is how to refactor this code, so when I need to change one thing I don't need to change on 40+ places.


Solution

  • As one of the design principle says:

    Encapsulate what varies

    So let's try to apply this principle.

    Assuming CreateValidIDCardsCommandModel and CreateValidIDCardsForeignerCommandModel has the same interface IRequest:

    public interface IRequest<T>
    {
        public T Id { get; set; }
    
        public T CountryId { get; set; }
    }
    
    public class CreateValidIDCardsCommandModel : IRequest<int>
    {
        public int Id { get; set; }
    
        public string Institution { get; set; }
    
        public int Total { get; set; }
    
        public int CountryId { get; set; }
    }
    
    public class CreateValidIDCardsForeignerCommandModel : IRequest<int>
    {
        public int Id { get; set; }
    
        public int Total { get; set; }
    
        public string Citizenship { get; set; }
    
        public int CountryId { get; set; }
    }
    

    And it can be concluded that ValidIdcardForeigner and ValidIdcard share the same interface as it is used in the following code snippet

    _context.ValidIdcards.Add(newItem);
    

    So we can reduce coupling in Handle() method by adding parameter U item. So we delegated creation of object to client of Handle method:

    public async Task<T> Handle<T, U>(IRequest<T> request, U item, 
        CancellationToken cancellationToken)
    {
        var country = _context.Countries.Where(x =>
        x.Code == request.CountryID)).FirstOrDefault();
    
        _context.ValidIdcards.Add(Item);
        await _context.SaveChangesAsync();
    }
    

    By doing this we do not need to edit Handle method by adding new ValidIdcardForeigner or ValidIdcard.