The previous articles/videos have been on design patterns – good ways to solve common programming problems. This article/video will cover an anti-pattern – a bad way that programs are sometimes written. I’ll show you how to detect the anti-pattern, and how to fix it.
I call this anti-pattern “Procedural code in Object-Oriented code “.
Watch the video version here:
What it is
Procedural code is code that must run in a specific order.
Think of it like driving directions: Go to the third streetlight, turn right, drive 10 blocks, turn left, then turn right after the second stop sign. If you don’t follow all those steps, in the exact order they were given, you probably won’t arrive at your destination.
The same problem can happen in a program – the code is written so steps must be performed in a specific order. If the programmer forgets to include a step, or calls one out of order, the program may not produce the correct results.
How to eliminate this anti-pattern
To prevent this problem, object-oriented programming uses information hiding and encapsulation.
I’ll demonstrate how to convert procedural code to good OOP code, using a sample order entry program.
We’ll have a Customer class, which only has the customer ID, and a Boolean property, showing if the Customer is a Premium member. Premium members don’t pay shipping for their orders.
The Order class will hold the Customer and a list of LineItem objects.
The LineItem class holds an InventoryItem object in the order – its description, price, weight – and the quantity ordered.
Order.cs
using System.Collections.Generic;
namespace Engine
{
public class Order
{
public int ID { get; set; }
public Customer Customer { get; set; }
public List<LineItem> LineItems { get; set; }
public Order(int id, Customer customer)
{
ID = id;
Customer = customer;
LineItems = new List<LineItem>();
}
}
}
Customer.cs
namespace Engine
{
public class Customer
{
public int ID { get; set; }
public string Name { get; set; }
public bool IsPremiumMember { get; set; }
public Customer(int id, string name, bool isPremiumMember)
{
ID = id;
Name = name;
IsPremiumMember = isPremiumMember;
}
}
}
LineItem.cs
namespace Engine
{
public class LineItem
{
public InventoryItem Item { get; set; }
public int Quantity { get; set; }
public LineItem(InventoryItem item, int quantity)
{
Item = item;
Quantity = quantity;
}
}
}
InventoryItem.cs
namespace Engine
{
public class InventoryItem
{
public enum SalesTaxCategory
{
Normal,
Medicine,
Food,
Luxury
}
public int ID { get; set; }
public string Name { get; set; }
public SalesTaxCategory TaxCategory { get; set; }
public decimal Price { get; set; }
public decimal Weight { get; set; }
public InventoryItem(int id, string name, SalesTaxCategory taxCategory, decimal price, decimal weight)
{
ID = id;
Name = name;
TaxCategory = taxCategory;
Price = price;
Weight = weight;
}
}
}
After all the items are added to an order, the programmer needs to determine the total cost of the order. The total includes tax, and shipping (if the customer is not a “Premium” member).
Here is how it might be done, using procedural code:
TestPlacingOrders.cs
using Engine;
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace TestEngine
{
[TestClass]
public class TestPlacingOrders
{
[TestMethod]
public void Test_SimpleOrder()
{
// Pretend these values came from the database.
InventoryItem book =
new InventoryItem(1, "Learn C# in 30 Minutes", InventoryItem.SalesTaxCategory.Normal, (decimal)19.95,
(decimal)2.5);
InventoryItem ebook =
new InventoryItem(2, "Learn F# in 31 Minutes", InventoryItem.SalesTaxCategory.Normal, (decimal)9.95, 0);
InventoryItem energyDrink =
new InventoryItem(3, "Monster Energy Drink", InventoryItem.SalesTaxCategory.Food, (decimal)1.95,
(decimal)0.75);
Customer joe = new Customer(1, "Joe Smith", false);
// Create an order
Order order = new Order(1, joe);
order.LineItems.Add(new LineItem(book, 2));
order.LineItems.Add(new LineItem(ebook, 1));
order.LineItems.Add(new LineItem(energyDrink, 10));
// Calculate order total amounts
const decimal normalTaxRate = (decimal)0.08;
const decimal medicineTaxRate = (decimal)0.00;
const decimal foodTaxRate = (decimal)0.04;
const decimal luxuryTaxRate = (decimal)0.14;
decimal subTotal = (decimal)0.0;
decimal tax = (decimal)0.0;
decimal shippingFee = (decimal)0.0;
foreach(LineItem lineItem in order.LineItems)
{
decimal lineItemSubTotal = lineItem.Item.Price * lineItem.Quantity;
subTotal += lineItemSubTotal;
if(lineItem.Item.TaxCategory == InventoryItem.SalesTaxCategory.Medicine)
{
tax += lineItemSubTotal * medicineTaxRate;
}
else if(lineItem.Item.TaxCategory == InventoryItem.SalesTaxCategory.Food)
{
tax += lineItemSubTotal * foodTaxRate;
}
else if(lineItem.Item.TaxCategory == InventoryItem.SalesTaxCategory.Luxury)
{
tax += lineItemSubTotal * luxuryTaxRate;
}
else // Normal
{
tax += lineItemSubTotal * normalTaxRate;
}
if(!order.Customer.IsPremiumMember)
{
// Non-Premium customers pay 50 cents per pound for shipping.
shippingFee += lineItem.Item.Weight * lineItem.Quantity * (decimal)0.50;
}
}
decimal total = subTotal + tax + shippingFee;
// Check values
Assert.AreEqual((decimal)69.35, subTotal);
Assert.AreEqual((decimal)4.768, tax);
Assert.AreEqual((decimal)6.25, shippingFee);
Assert.AreEqual((decimal)80.368, total);
}
}
}
In the procedural example, the programmer calculates the subtotal, tax, shipping, and total – as local variables.
Notice that all the calculation logic is all done at the highest level of the class hierarchy.
This works correctly, but could lead to problems in the future.
You might decide to make a mobile app version of your program, so customers can place orders from their phones. If your company hires a new mobile app developer, and they look at the Order class, they won’t know the rules, and sequence, for calculating the order totals.
Or, you might start selling to customers in a different country. Those orders might not have tax applied, and might have a higher shipping cost. With this procedural code, you would need to find every place where the order totals are calculated and make changes for the new rules. It’s easy to miss one.
With a small sample like this, it’s not too difficult to find the place where the Total is being calculated. But, when your solutions have dozens of projects, and hundreds of thousands of lines of code, this becomes a serious problem.
How to detect this anti-pattern
One way is too look for any place in your code where a variable, or property, is being assigned a value – and all the properties to compute that value (the ones on the right side of the “=” sign), are from another class. Another place where you might see this anti-pattern is in large “if-else”, or “switch” statements.
When you see this, you can probably move those calculations into get-only properties, of the other class.
Here is what the code looks like after refactoring.
Order.cs
using System.Collections.Generic;
using System.Linq;
namespace Engine
{
public class Order
{
public int ID { get; set; }
public Customer Customer { get; set; }
public List<LineItem> LineItems { get; set; }
public decimal SubTotal
{
get { return LineItems.Sum(x => x.SubTotal); }
}
public decimal Tax
{
get { return LineItems.Sum(x => x.SubTotal * x.Item.TaxRate); }
}
public decimal ShippingFee
{
get { return Customer.IsPremiumMember ? 0 : LineItems.Sum(x => x.ShippingFee); }
}
public decimal Total
{
get { return SubTotal + Tax + ShippingFee; }
}
public Order(int id, Customer customer)
{
ID = id;
Customer = customer;
LineItems = new List<LineItem>();
}
}
}
Customer.cs
namespace Engine
{
public class Customer
{
public int ID { get; set; }
public string Name { get; set; }
public bool IsPremiumMember { get; set; }
public Customer(int id, string name, bool isPremiumMember)
{
ID = id;
Name = name;
IsPremiumMember = isPremiumMember;
}
}
}
LineItem.cs
namespace Engine
{
public class LineItem
{
public InventoryItem Item { get; set; }
public int Quantity { get; set; }
public decimal SubTotal
{
get { return Item.Price * Quantity; }
}
public decimal ShippingFee
{
get { return Item.Weight * Quantity * (decimal)0.50; }
}
public LineItem(InventoryItem item, int quantity)
{
Item = item;
Quantity = quantity;
}
}
}
InventoryItem.cs
namespace Engine
{
public class InventoryItem
{
public enum SalesTaxCategory
{
Normal,
Medicine,
Food,
Luxury
}
public int ID { get; set; }
public string Name { get; set; }
public SalesTaxCategory TaxCategory { get; set; }
public decimal Price { get; set; }
public decimal Weight { get; set; }
public decimal TaxRate
{
get
{
if(TaxCategory == SalesTaxCategory.Medicine)
{
return (decimal)0.00;
}
if(TaxCategory == SalesTaxCategory.Food)
{
return (decimal)0.04;
}
if(TaxCategory == SalesTaxCategory.Luxury)
{
return (decimal)0.14;
}
return (decimal)0.08; // Normal
}
}
public InventoryItem(int id, string name, SalesTaxCategory taxCategory, decimal price, decimal weight)
{
ID = id;
Name = name;
TaxCategory = taxCategory;
Price = price;
Weight = weight;
}
}
}
TestPlacingOrders.cs
using Engine;
using Microsoft.VisualStudio.TestTools.UnitTesting;
namespace TestEngine
{
[TestClass]
public class TestPlacingOrders
{
[TestMethod]
public void Test_SimpleOrder()
{
// Pretend these values came from the database.
InventoryItem book =
new InventoryItem(1, "Learn C# in 30 Minutes", InventoryItem.SalesTaxCategory.Normal, (decimal)19.95,
(decimal)2.5);
InventoryItem ebook =
new InventoryItem(2, "Learn F# in 31 Minutes", InventoryItem.SalesTaxCategory.Normal, (decimal)9.95, 0);
InventoryItem energyDrink =
new InventoryItem(3, "Monster Energy Drink", InventoryItem.SalesTaxCategory.Food, (decimal)1.95,
(decimal)0.75);
Customer joe = new Customer(1, "Joe Smith", false);
// Create an order
Order order = new Order(1, joe);
order.LineItems.Add(new LineItem(book, 2));
order.LineItems.Add(new LineItem(ebook, 1));
order.LineItems.Add(new LineItem(energyDrink, 10));
// Check values.
// Notice that nothing needs to be calculated in this code.
// All values are calculated in the business classes.
Assert.AreEqual((decimal)69.35, order.SubTotal);
Assert.AreEqual((decimal)4.768, order.Tax);
Assert.AreEqual((decimal)6.25, order.ShippingFee);
Assert.AreEqual((decimal)80.368, order.Total);
}
}
}
In this version of the code, the calculation logic is moved “down” the hierarchy, and the results are “pulled up”.
If a new programmer looks at this code, and wants to see how the Total is calculated, the logic is much more obvious.
Where I’ve found it useful
I usually see this anti-pattern in large programs, that have been worked on by many programmers, or many years.
Code gets copied and pasted throughout the program. When a change to the rules happens, it doesn’t always get updated every place where the code was copied. When you find these differences, you’ll often hear that it’s a known problem – but it only happens in certain situations, so no one has ever investigated the root problem.
By fixing this anti-pattern, your programs will be more reliable, since it’s using the same rules everyplace. It will also be easier and faster to modify your programs. You’ll only need to make the change in one place.
Leave a Comment