Skip to main content
edited title
Link
Margus
  • 914
  • 4
  • 12

How to clean up a method containing multiple responsibilities? Returning rental invoice data transfer object

edited title
Link
Margus
  • 914
  • 4
  • 12

How to clean up code that does way too many thingsa method containing multiple responsibilities?

Source Link
Margus
  • 914
  • 4
  • 12

How to clean up code that does way too many things

When trying to follow single responsibility principle, I often find myself in a position where there is so much to do that I do not know where to start. Maybe there is there a rule of thumb that I could use?

A concrete example originating from my test project.

    // doing way too many things
    public InvoiceDto GetInvoice(CartDto cart)
    {
        if (cart == null)
            throw new ArgumentNullException(nameof(cart));                                      // validation

        var cartequipments = _carts.GetById(cart.CartId).CartEquipments.ToList();               // quering
        var rentals = cartequipments.Select(o =>                                                // mapping
            new RentalDto                                                                       // object creation
            {
                Name = o.Equipment.EquipmentName,
                Price = _mapPriceCalculatorLogic                                                // buisiness logic
                    .Create(o.Equipment.EquipmentType)
                    .Calculate(o.RentDurationDays)
            }).ToList();
        var loyaltyPoints = _loyaltyPointsService                                               // buisiness logic
            .GetLoyaltyPoints(cartequipments.Select(o => o.Equipment.EquipmentType)); 
        var total = rentals.Sum(o => o.Price);                                                  // aggregation

        var invoiceDto = new InvoiceDto                                                         // object creation 
        {
            Title = $"Invoice id : {cart.CartId}",
            Rentals = rentals,
            LoyaltyPoints = loyaltyPoints,
            TotalPrice = total
        };

        return invoiceDto;
    }

My question is how would you clean this up and if possible what would the result look like (helper methods sources not needed).