2
\$\begingroup\$

I am quite new to DI and its patterns so I decided to place my code for some review.

I am working on an digital map application using .NET Core 2.0. I am using the standard DI framework of ASP.NET Core. On the map different containers (the ones for transfering building and waste materials) are displayed. Every container has a status, which can be Old, Takeable, BeingFilled or Idle. Let's see first the corresponding enums: (do not care about the code parts relating to JsonConverter...)

[JsonConverter(typeof(StringEnumConverter))]
public enum Status
{
    [EnumMember(Value = "Idle")]
    Idle,
    [EnumMember(Value = "BeingFilled")]
    BeingFilled,
    [EnumMember(Value = "Takeable")]
    Takeable,
    [EnumMember(Value = "Old")]
    Old
}

I also have a custom mapping service class which is used to create ViewModels from domain objects. First let's see how my ContainerMapViewModel looks like:

public class ContainerMapViewModel
{
    public string CompoundId { get; set; }
    public Location Location { get; set; }
    public string FullAddress { get; set; }
    public string PhoneNumber { get; set; }
    public string LayDownDate { get; set; }
    public string LayedDownBy { get; set; }
    public string TakeUpDate { get; set; }
    public string Others { get; set; }
    public Status Status { get; set; }
}

Then let's see the implementation of custom mapper service:

public class ContainerToMapViewModelMappingService : IContainerMappingService<ContainerMapViewModel>
{
    private readonly ContainerDataTransformer _dataTransformer = new ContainerDataTransformer();
    private readonly IDateTime _dateTime;

    private ContainerStatusGetter _statusGetter;

    public ContainerToMapViewModelMappingService(IDateTime dateTime)
    {
        _dateTime = dateTime;
    }

    public ContainerMapViewModel Map(Container container)
    {
        _statusGetter = new ContainerStatusGetter(_dateTime, container.Order);

        var viewModel = new ContainerMapViewModel()
        {
            CompoundId = _dataTransformer.CreateCompoundId(container),
            Location = container.Location,
            FullAddress = _dataTransformer.CreateFullAddress(container),
            PhoneNumber = _dataTransformer.GetPhoneNumber(container.Order),
            LayDownDate = _dataTransformer.GetLayDownDate(container.Order),
            LayedDownBy = container.LayedDownBy.FirstName,
            TakeUpDate = _dataTransformer.GetTakeUpDate(container.Order),
            Others = _dataTransformer.GetOthers(container.Order),
            Status = _statusGetter.Get()
        };

        return viewModel;
    }
}

As you can see I also have a ContainerStatusGetter which is responsible for getting the status of a container. On the first hand I extracted this status getter in order to follow the Single Responsibility Principle. On the second hand in the ContainerStatusGetter I use IDateTime in order to mock the datetime easily and test my ContainerStatusGetter with unit tests:

public interface IDateTime
{
    DateTime Now { get; }
}

public class ContainerStatusGetter
{
    private const int NumberOfAllowedDaysToStay = 3;

    private readonly IDateTime _dateTime;
    private readonly Order _order;

    public ContainerStatusGetter(IDateTime dateTime, Order order)
    {
        _dateTime = dateTime;
        _order = order;
    }


    public Status Get()
    {
        return _order != null
            ? GetStatus()
            : Status.Idle;
    }

    private Status GetStatus()
    {
        return TakeUpDateIsNotDecidedAndContainerStaysMoreThanAllowedDays() 
            ? Status.Old 
            : IsTakeAble();
    }

    private bool TakeUpDateIsNotDecidedAndContainerStaysMoreThanAllowedDays()
    {
        return _order.TakeUpDate.Equals(null) && (_dateTime.Now - _order.LayDownDate).TotalDays >= NumberOfAllowedDaysToStay;
    }

    private Status IsTakeAble()
    {
        return _dateTime.Now.Equals(_order.TakeUpDate)
            ? Status.Takeable
            : Status.BeingFilled;
    }
}

Just one unit test from the many:

public class ContainerStatusGetterTest
{

    [Fact]
    public void NoOrder_GetStatus_Idle()
    {
        //Arrange
        var mockedDateTime = new Mock<IDateTime>();
        mockedDateTime.Setup(mock => mock.Now).Returns(dateTime);

        var getter = new ContainerStatusGetter(mockedDateTime.Object, null);

        //Act
        var status = getter.Get();

        //Assert
        status.Should().Be(Status.Idle);
    }

My question is the following: Is it a good way to inject the IDateTime into my mapping service then pass it to my ContainerStatusGetter? As far as I can judge there is a code-smell here, because the IDateTime is only used in the ContainerStatusGetter. Well I could only inject the IDateTime into my ContainerStatusGetter but the problem is, that it also needs the Order object which comes from Container domain object, so it is not possible to set up this dependency in the ConfigureServices method in Startup.cs. What do you guys think, what is the best way to do this? Do you have any other recommendation regarding to my code? Maybe shall I use some other DI framework like Autofac in order to support other DI pattern like method-DI? I read that it can be also a good option, if my class can exist without that dependency.

\$\endgroup\$
  • 1
    \$\begingroup\$ @t3chb0t I thought that those codes are not relevant to my question at all, so i decided not to put in the code. But I can do it, I finish with working then I am going to put those code-parts as well, thanks for your remark. \$\endgroup\$ – mirind4 Aug 23 '17 at 6:21
  • \$\begingroup\$ @t3chb0t I put the whole code to my snippets. Or it is already closed? :O \$\endgroup\$ – mirind4 Aug 23 '17 at 17:16
  • \$\begingroup\$ Great ;-) It's not closed yet but even if it was, it doesn't have to be forever. Questions get reopened if they are fixed ;-) Now that the code is complete I no longer see a reason for the close-vote so I retract it. \$\endgroup\$ – t3chb0t Aug 23 '17 at 17:20
  • \$\begingroup\$ @t3chb0t oh, that's right, sounds good then :) Cheers! \$\endgroup\$ – mirind4 Aug 23 '17 at 17:22
3
\$\begingroup\$

Per my understanding: are you trying to use ContainerStatusGetter as an AggregateRoot? If so, then you need an AggregateRepository that get the IDateTime service from the constructor, and has a GetByOrder([Order Key or Handler]) method that instantiates the Aggregate passing the Service and the Bussiness Model.

public interface IDateTime
{
    DateTime Now { get; }
}

public class OrderAggregateRepository
{
    private readonly IDateTime _dateTime;
    private readonly IOrderRepository _repository;

    public OrderAggregateRepository(IDateTime dateTime, IOrderRepository repository)
    {
        _dateTime = dateTime;
        _repository = repository;
    }

    public OrderAggregateRoot GetByOrderID(int orderId)
    {
        var order = _repository.GetByID(orderId); // Resolve the Domain Entity
        return new OrderAggregateRoot(_dateTime, order);
    }
}

public class OrderAggregateRoot
{
    private const int NumberOfAllowedDaysToStay = 3;

    private readonly IDateTime _dateTime;
    private readonly Order _order;

    public OrderAggregateRoot(IDateTime dateTime, Order order)
    {
        _dateTime = dateTime;
        _order = order;
    }

    public Status Get()
    {
        return _order != null
            ? GetStatus()
            : Status.Idle;
    }

    private Status GetStatus()
    {
        return TakeUpDateIsNotDecidedAndContainerStaysMoreThanAllowedDays() 
            ? Status.Old 
            : IsTakeAble();
    }

    private bool TakeUpDateIsNotDecidedAndContainerStaysMoreThanAllowedDays()
    {
        return _order.TakeUpDate.Equals(null) && (_dateTime.Now - _order.LayDownDate).TotalDays >= NumberOfAllowedDaysToStay;
    }

    private Status IsTakeAble()
    {
        return _dateTime.Now.Equals(_order.TakeUpDate)
            ? Status.Takeable
            : Status.BeingFilled;
    }
}

This is a high overview of how the Domain layer interacts to with the Data, and how to retrieve the AggregateRoot for an Entity.

Look forward to checking online about different approach of the Entity-Aggregate relation. Here you can start to your DDD journey.

Hope this help!

\$\endgroup\$
  • \$\begingroup\$ Thank you very much for your answer! So far I did not know what AggregateRoot is, just read about it. This DDD looks promising I will learn more about it! So just to make the things a bit clear because it is a bit difficult to understand what you say without code example. (If you would provide some I would really appreciate it, thanks) So I make a OrderAggregateRepository which has an injected IDateTime constructor parameter, and an GetByOrder method which instantiates the Aggregate object which in this case the Order. Then the repository should be used in this class.Is it correct? :) \$\endgroup\$ – mirind4 Aug 23 '17 at 17:31
  • \$\begingroup\$ Check the updates \$\endgroup\$ – Miguel Aug 23 '17 at 17:57
  • \$\begingroup\$ @Migual Thank you very much! This is what I really wanted: a nice code example and a good place to start with DDD! Cheers ;) \$\endgroup\$ – mirind4 Aug 23 '17 at 18:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.