Chapter 6: Improving Class Quality

In this chapter we will see how to refactor our code to be more cohesive and less coupled.

Refactoring classes with low-cohesion

Clearly single responsibility (and the heading of this section) suggests that we should refactor Invoice into multiple classes. But, how do we do that given our current scenario? The designer of this class has obviously thought that the functionality of Invoice included rendering a readable invoice, so how do we make a clean separation? Fortunately, the lack of cohesiveness gives us our separation points. What makes an invoice an invoice doesn't include those fields that deal solely with rendering a readable invoice. Fields like HeaderFont, HeaderBrush, HeaderText, HeaderLocation, FooterFont, FooterBrush, FooterText, FooterLocation, InvoiceBodyFont, InvoiceBodyBrush, LineItemLocation, InvoiceBustTotalLocation, InvoiceTaxLocation, and InvoiceGrandTotalLocation all deal with just the rendering responsibility.

The Invoice class is modeling a real-world invoice. When you hold an invoice in your hand or view it on the screen, it's already rendered. In the real-world we'd never think that a responsibility of rendering an invoice would be a responsibility of the invoice itself.

We know we want to retain our original Invoice class and we want to move the rendering responsibility to a new class. This new class will encapsulate the responsibility of rendering an invoice. Since this new class will take an Invoice object and help another class produce something useful, we can consider this an invoice rendering service.

In order to refactor our existing Invoice class to a new Invoice rendering service, we start with a new InvoiceRenderingService class and move the HeaderFont, HeaderBrush, HeaderText, HeaderLocation, FooterFont, FooterBrush, FooterText, FooterLocation, InvoiceBodyFont, InvoiceBodyBrush, LineItemLocation, InvoiceBustTotalLocation, InvoiceTaxLocation, and InvoiceGrandTotalLocation fields to the InvoiceRenderingService. Next, we move the GenerateReadableInvoice method to the InvoiceRenderingService. At this point, we basically have a functional class, but since the InvoiceRenderingService method was on the Invoice classes, the other properties that the GenerateReadableInvoice uses need an Invoice object reference-effectively changing it from "this" to a parameter to the GenerateReadableInvoice method. Since the original Invoice class was never expected to be used externally like this, we need to add a CalculateGrandTotal method that delegates to the invoiceGrandTotalStrategy object. The result is something like the following:

/// <summary>
/// Encapsulates a service to render an invoice
/// to a Graphics device.
/// </summary>
public class InvoiceRenderingService
{
    public void GenerateReadableInvoice(Invoice invoice,Graphics graphics)
    {
        graphics.DrawString(HeaderText,HeaderFont,HeaderBrush,HeaderLocation);
        float invoiceSubTotal = 0;
        PointF currentLineItemLocation = LineItemLocation;
        foreach (InvoiceLineItem invoiceLineItem in invoice.InvoiceLineItems)
        {
            float lineItemSubTotal =Invoice.CalculateLineItemSubTotal(invoiceLineItem);
            graphics.DrawString(invoiceLineItem.Description,InvoiceBodyFont,InvoiceBodyBrush,currentLineItemLocation);
            currentLineItemLocation.Y +=InvoiceBodyFont.GetHeight(graphics);
            invoiceSubTotal += lineItemSubTotal;
        }
        float invoiceTotalTax =invoice.CalculateInvoiceTotalTax(invoiceSubTotal);
        float invoiceGrandTotal =invoice.CalculateGrandTotal(invoiceSubTotal,invoiceTotalTax);
        Invoice.CalculateInvoiceGrandTotal(invoiceSubTotal,invoiceTotalTax);
        graphics.DrawString(String.Format("Invoice SubTotal: {0}",invoiceGrandTotal - invoiceTotalTax),InvoiceBodyFont, InvoiceBodyBrush,
            InvoiceSubTotalLocation);
        graphics.DrawString(String.Format("Total Tax: {0}",invoiceTotalTax), InvoiceBodyFont,InvoiceBodyBrush, InvoiceTaxLocation);
        graphics.DrawString(String.Format("Invoice Grand Total: {0}",invoiceGrandTotal), InvoiceBodyFont,InvoiceBodyBrush, InvoiceGrandTotalLocation);
        graphics.DrawString(FooterText,FooterFont,FooterBrush,FooterLocation);
    }
    public string HeaderText { get; set; }
    public Font HeaderFont { get; set; }
    public Brush HeaderBrush { get; set; }
    public RectangleF HeaderLocation { get; set; }
    public string FooterText { get; set; }
    public Font FooterFont { get; set; }
    public Brush FooterBrush { get; set; }
    public RectangleF FooterLocation { get; set; }
    public Font InvoiceBodyFont { get; set; }
    public Brush InvoiceBodyBrush { get; set; }
    public Point LineItemLocation { get; set; }
    public RectangleF InvoiceSubTotalLocation { get; set; }
    public RectangleF InvoiceTaxLocation { get; set; }
    public RectangleF InvoiceGrandTotalLocation { get; set; }
}

Alternatively, the whole use of invoiceGrandTotalStrategy can be moved into the InvoiceRenderingService-which is a better design decision.