Improve The Code

For today I’ve decided to try a more interactive format: First, I’ll give you a short listing of C# programming screaming out for improvement—classes lacking encapsulation and abstraction, holding data but no behaviour. Then it’s your turn to try and change the code to be more object-oriented. You can send me your attempts to olaf@codecoach.co.nz, and I’ll publish and discuss any notable efforts in Monday’s post when I also reveal my answer. I must receive your email by Friday evening to make it into Monday’s post.

Here is an opportunity for you to show off your abilities or learn something—you win either way.

The best effort I receive gets a free coffee (as long as you’re in Auckland)
once we’re out of lockdown. If the winner is not local or lockdown remains in place, I offer a 30-minute Google Meet Q & A session. Something’s been bugging you about how to become a better programmer; here’s your chance to ask the Code Coach. Not bad, eh?!

Here is the unit test:

  using System.Collections.Generic;
  using System.Linq;
  using Xunit;
  using FluentAssertions;

  namespace NotOO
  {
     public class SalesInvoiceTests
     {
        [Fact]
        public void Add_Lines_To_SalesInvoice_And_Verify_They_Have_Been_Added()
        {
           // Setup products
           var apple = new Product
           {
              Name = "Apple",
              UnitPrice = 0.35m
           };
           var banana = new Product
           {
              Name = "Banana",
              UnitPrice = 0.75m
           };

           // Setup sales invoice lines
           var lines = new List<SalesInvoiceLine>();
           var line1 = new SalesInvoiceLine
           {
              Product = apple,
              Quantity = 4,
              Subtotal = 4 * apple.UnitPrice
           };
           lines.Add(line1);

           var line2 = new SalesInvoiceLine
           {
              Product = banana,
              Quantity = 3,
              Subtotal = 3 * banana.UnitPrice
           };
           lines.Add(line2);

           // Set up sales invoice
           var invoice = new SalesInvoice
           {
              Lines = lines,
              LineCount = lines.Count,
              Total = lines.Sum(l => l.Subtotal)
           };

           // Verify sales invoice has added the sales invoice lines.
           invoice.Total.Should().Be(3.65m);
           invoice.LineCount.Should().Be(2);
        }
     }
  }

Note: This unit test is only testing behaviour that has been set up inside itself! Check out where the Total and Subtotal calculations are done. This behaviour should be moved to the classes. 

And here are the definitions for SalesInvoice, SalesInvoiceLine and Product:

  using System.Collections.Generic;

  namespace NotOO
  {
     public class SalesInvoice
     {
        public decimal Total { get; set; }
        public int LineCount { get; set; }
        public List<SalesInvoiceLine> Lines { get; set; }
     }

     public class SalesInvoiceLine
     {
        public Product Product { get; set; }
        public int Quantity { get; set; }
        public decimal Subtotal { get; set; }
     }

     public class Product
     {
        public string Name { get; set; }
        public decimal UnitPrice { get; set; }
     }
  }

Your task, should you choose to accept it, is to turn these simplistic class definitions into proper objects, containing not only data but also behaviour. The unit test should become significantly shorter and more readable. Please send me your improved unit test as well.

Good luck!

Until Monday.