Deal With Technical Debt and Slop
At this point, you have a process to handle most programming tasks you’d be given on an average day. From bug-fixing, to small features, to larger features, you should be able to deliver results quickly and easily.
Despite your best efforts, code can begin to rot and become “crufty”. Over time, the design of certain parts of our application will begin to hinder our ability to change it. Many developers call code like this technical debt. The reality is that sometimes sloppy code is just sloppy and could’ve been easily avoided. I call this slop, and this chapter is about understanding the difference.
Slop: Code to Fix Now
You’re under the gun to deliver a feature quickly and you’ve just gotten your acceptance-level test to pass. Your code has unit test coverage and you feel confident that the system works. But, to do so, you had to copy and paste a lot of business logic, your variable names aren’t very good, and you’ve created a few new public APIs that are poorly documented and counter-intuitive. Should you ship this code?
No.
The reason is that you can fix these problems before the code has a chance to infect the rest of the system. All of these issues are not only fixable, but quickly fixable. Removing copy and paste code, renaming variables, and documenting APIs is straightforward, and can often be done in less time than it takes to discuss whether or not you should even do them! Depending on your language and toolchain, you could fix some of these issues with the click of a button.
Code that has these sorts of issues is called slop because it’s sloppy and can be fixed quickly. Slop is also indicated by poor test coverage, missing error handling, inconsistent use of logging, or a lack of validation of assumptions. Slop exists mainly because you didn’t spend enough time refactoring. Slop is complexity that you’ve introduced and that you can easily fix.
Don’t feel too bad about it. The process of writing software is a human one and no developer can churn out perfectly clean code every time. The entire reason we separate refactoring into its own step is to allow us to focus on issues just like these. So when you see slop, take a few minutes to clean it up.
Let’s look at an example.
Suppose we have a Customer class, a Product class, and an Order class. Each of these represents analogous domain objects: A customer purchases a product, creating an order representing that purchase.
Suppose that the system’s Order class has a method that can determine if the customer should be able to purchase a particular Product for free, based on the customer’s existing store credit, other discounts, and the price of the product. That code looks like so (if you aren’t familiar with Ruby, see appendix in the book for a very brief overview of how to read Ruby code):
class Product
def price
@price
end
end
class Order
# ...
def free?
(self.customer.store_credit +
self.discounts_applied) >= self.product.price
end
end
The method free? returns a boolean based on comparing the sum of the order’s customer’s store_credit and the order’s discounts against the price of the order’s product. This is the current state of the system.
You now need to implement a new feature where the customer is shown that a product they might purchase would be free, based on the store credit they have. Further, the customer might have applied a discount code during the ordering process, so we want to take that into account as well.
Because the customer needs to see this information before a purchase is created, you can’t re-use the free? method on Order directly because there is no order yet.
After writing your tests, you get things working as quickly as you can which, in this case, means copying the code from Order and tweaking it slightly.
class Product
# ...
def free_for_customer?(customer,code)
discounts = 0
if !code.nil?
discounts = code.discount_amount
end
(customer.store_credit + discounts) >=
self.price
end
end
The tests pass, and the feature technically works, but it’s less than ideal. Not only have you copied code from somewhere else, you’ve modified it slightly so the duplicated business logic isn’t apparent. Anyone creating future features related to discounting and store credit will have to change both of these blocks of code, assuming they realize that both duplicate the same business logic. Further, the variable code is not very well-named. It should be discount_code, but you were in a hurry.
Many developers, if they are feeling pressured to complete their work, would call this an acceptable compromise, promise to fix it later, and ship it. This code is slop and, as a senior developer, you shouldn’t ship it. You only need a few minutes to clean it up, which will save much more time than that down the road.
All you need to do is extract a method to encapsulate the discounting logic somewhere accessible to both Product and Order. Since every Order has a Product, you can put this new method in Product. You call it purchase_free? and have it take the customer and any discounts that were applied.
class Product
def free_for_customer?(customer,discount_code)
discounts = 0
if !discount_code.nil?
discounts = discount_code.discount_amount
end
purchase_free?(customer,discounts)
end
def purchase_free?(customer,discounts)
(customer.store_credit + discounts) >=
self.price
end
end
class Order
def free?
self.product.purchase_free?(
self.customer,
self.discounts_applied)
end
end
The copy and paste code is now gone (as is the bad variable name) and it took maybe five minutes to fix. There is now exactly one place where the logic related to discounting lives and no one would be confused by how it works. This code is clean, maintainable, and shippable. It is no longer slop. Since all of the tests are still passing, you can be sure that this change is a good one and didn’t break anything.
That being said, there are still some issues with this code. Does discounting logic really belong in the Product class? It’s a convenient place to put it, but it seems out of place. If a future feature needs to calculate a discount, but doesn’t have a Product instance, it’s going to be hard to re-use this logic. Imaginary features like this can lead to over-engineering and the phrase “You Aren’t Gonna Need It” can keep your imagination in check.
Still, what if such a feature did come up? It’s hard to argue that our code is now slop simply because of a new requirement. The reality is that we made a design decision based on the state of the world at the time and, if world changes, our once clean code would make it difficult to change the system. This is a form of technical debt.