Chapter 6: Improving Class Quality

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

Class cohesion

Object-orientation brings extra cohesive abilities to the programmer. The programmer has the ability to relate code together within a class. Other code can use the code within a class, but only through its defined boundaries (the class's methods and properties).

In object-oriented design, cohesion is generally much more than simply code contained within a class. Object-oriented cohesiveness goes beyond the physical relation of code within a class and deals with the relation of meaning of the code within a class.

Object-oriented language syntax allows the programmer to freely relate code to other code through a class definition, but this doesn't mean that code is cohesive.

For example, let's revisit our Invoice class so far.

/// <summary>
/// Invoice class to encapsulate invoice line items
/// and drawing
/// </summary>
public class Invoice
{
    private IInvoiceGrandTotalStrategy invoiceGrandTotalStrategy;
    public Invoice(IEnumerable<InvoiceLineItem> invoiceLineItems,
    IInvoiceGrandTotalStrategy invoiceGrandTotalStrategy)
    {
        InvoiceLineItems = new List<InvoiceLineItem>(invoiceLineItems);
        this.invoiceGrandTotalStrategy = invoiceGrandTotalStrategy;
    }
    private List<InvoiceLineItem> InvoiceLineItems
    {
        get;
        set;
    }
    public void GenerateReadableInvoice(Graphics graphics)
    {
        graphics.DrawString(HeaderText, HeaderFont, HeaderBrush, HeaderLocation);
        float invoiceSubTotal = 0;
        PointF currentLineItemLocation = LineItemLocation;
        foreach (InvoiceLineItem invoiceLineItem in InvoiceLineItems)
        {
            float lineItemSubTotal =CalculateLineItemSubTotal(invoiceLineItem);
            graphics.DrawString(invoiceLineItem.Description, InvoiceBodyFont, InvoiceBodyBrush, currentLineItemLocation);
            currentLineItemLocation.Y +=InvoiceBodyFont.GetHeight(graphics);
            invoiceSubTotal += lineItemSubTotal;
        }
        float invoiceTotalTax = CalculateInvoiceTotalTax(invoiceSubTotal);
        float invoiceGrandTotal = invoiceGrandTotalStrategy.CalculateGrandTotal(invoiceSubTotal,invoiceTotalTax);
        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 static float CalculateInvoiceGrandTotal(float invoiceSubTotal, float invoiceTotalTax)
    {
        float invoiceGrandTotal = invoiceTotalTax + invoiceSubTotal;
        return invoiceGrandTotal;
    }
    public float CalculateInvoiceTotalTax(float invoiceSubTotal)
    {
        float invoiceTotalTax =(float)((Decimal)invoiceSubTotal *(Decimal)TaxRate);
        return invoiceTotalTax;
    }
    public static float CalculateLineItemSubTotal(InvoiceLineItem invoiceLineItem)
    {
        float lineItemSubTotal = (float)((decimal)(invoiceLineItem.Price - invoiceLineItem.Discount) * (decimal)invoiceLineItem.Quantity);
        return lineItemSubTotal;
    }
    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 float TaxRate { 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; }
}

We have an operational Invoice class. It does some things, and they work. But, our Invoice class isn't very cohesive. The Invoice class has distinct groups of fields. Some are for the state of an invoice and some are for generating a readable invoice. Methods that deal with the behavior and attributes of an invoice don't use the fields that deal with generating a readable invoice.

Our Invoice class is implementing two distinct tasks: managing invoice state and generating a readable invoice. The data required to generate a readable invoice (over and above the data shown on an invoice) isn't used by Invoice when not generating a readable invoice.

Our Invoice class can be said to have multiple responsibilities: the responsibility of managing state and the responsibility of generating a readable invoice. What makes an invoice an invoice may be fairly stable; we may occasionally need to add, remove, or change fields that store data contained in an invoice. But, the act of displaying a readable invoice may be less stable: it may change quite frequently. Worse still, the act of displaying a readable invoice may depend on the platform it is running on.