Skip to main content
Added sample
Source Link
psaxton
  • 892
  • 5
  • 9

Your code is well formatted and easy to read. Congratulations.

With regards to SRP, I believe your code complies just fine. Each dependency: the cart, rentals, and loyalty points is retrieved with a single call. The mapping for those DTO are isolated in a small single purpose anonymous method. The single responsibility for the method matches its name GetInvoiceDTOGetInvoice.

If your uncomfortable with the idea of anonymous methods being separate from the method where they are defined, consider creating private static named methods in this method's class and referencing them.

The only SRP violation I'm concerned with is the invoice DTO. I would recommend using the raw cart id and letting your presentation code format the title.

Here's an example of how I may clean it up:

public InvoiceDto GetInvoice(CartDto cart)
{
    if (cart == null)
        throw new ArgumentNullException(nameof(cart));

    var cartequipments = _carts.GetById(cart.CartId).CartEquipments.ToList();
    var rentals = cartequipments.Select(o => MapEquipmentToRentalDto).ToList();
    var loyaltyPoints = _loyaltyPointsService
        .GetLoyaltyPoints(cartequipments.Select(o => o.Equipment.EquipmentType)); 
    var total = rentals.Sum(o => o.Price);

    var invoiceDto = new InvoiceDto 
    {
        InvoiceId = cart.CartId,
        Rentals = rentals,
        LoyaltyPoints = loyaltyPoints,
        TotalPrice = total
    };

    return invoiceDto;
}

private static RentalDto MapEquipmentToRentalDto(CartEquipment o)
{
    return new RentalDto 
    {
        Name = o.Equipment.EquipmentName,
        Price = _mapPriceCalculatorLogic
                .Create(o.Equipment.EquipmentType)
                .Calculate(o.RentDurationDays)
    };
}

Your code is well formatted and easy to read. Congratulations.

With regards to SRP, I believe your code complies just fine. Each dependency: the cart, rentals, and loyalty points is retrieved with a single call. The mapping for those DTO are isolated in a small single purpose anonymous method. The single responsibility for the method matches its name GetInvoiceDTO.

If your uncomfortable with the idea of anonymous methods being separate from the method where they are defined, consider creating private static named methods in this method's class and referencing them.

The only SRP violation I'm concerned with is the invoice DTO. I would recommend using the raw cart id and letting your presentation code format the title.

Your code is well formatted and easy to read. Congratulations.

With regards to SRP, I believe your code complies just fine. Each dependency: the cart, rentals, and loyalty points is retrieved with a single call. The mapping for those DTO are isolated in a small single purpose anonymous method. The single responsibility for the method matches its name GetInvoice.

If your uncomfortable with the idea of anonymous methods being separate from the method where they are defined, consider creating private static named methods in this method's class and referencing them.

The only SRP violation I'm concerned with is the invoice DTO. I would recommend using the raw cart id and letting your presentation code format the title.

Here's an example of how I may clean it up:

public InvoiceDto GetInvoice(CartDto cart)
{
    if (cart == null)
        throw new ArgumentNullException(nameof(cart));

    var cartequipments = _carts.GetById(cart.CartId).CartEquipments.ToList();
    var rentals = cartequipments.Select(o => MapEquipmentToRentalDto).ToList();
    var loyaltyPoints = _loyaltyPointsService
        .GetLoyaltyPoints(cartequipments.Select(o => o.Equipment.EquipmentType)); 
    var total = rentals.Sum(o => o.Price);

    var invoiceDto = new InvoiceDto 
    {
        InvoiceId = cart.CartId,
        Rentals = rentals,
        LoyaltyPoints = loyaltyPoints,
        TotalPrice = total
    };

    return invoiceDto;
}

private static RentalDto MapEquipmentToRentalDto(CartEquipment o)
{
    return new RentalDto 
    {
        Name = o.Equipment.EquipmentName,
        Price = _mapPriceCalculatorLogic
                .Create(o.Equipment.EquipmentType)
                .Calculate(o.RentDurationDays)
    };
}
Source Link
psaxton
  • 892
  • 5
  • 9

Your code is well formatted and easy to read. Congratulations.

With regards to SRP, I believe your code complies just fine. Each dependency: the cart, rentals, and loyalty points is retrieved with a single call. The mapping for those DTO are isolated in a small single purpose anonymous method. The single responsibility for the method matches its name GetInvoiceDTO.

If your uncomfortable with the idea of anonymous methods being separate from the method where they are defined, consider creating private static named methods in this method's class and referencing them.

The only SRP violation I'm concerned with is the invoice DTO. I would recommend using the raw cart id and letting your presentation code format the title.