Developing as a Developer: Code Smells – Object-Orientation Abusers
The category of Object-Orientation Abusers encompass smells where the code fails in it's implementation of object-oriented design principles and best practices. This includes but is not limited to using conditionals or type codes instead of subclasses, failing to leverage inheritance, or inappropriately scoping data (misuse of encapsulation) (Mäntylä & Lassenius, 2006).
Keep in mind that the refactoring solutions presented are not unique to the code smells presented and likely also appeared (or will appear) in other posts in this series. To me this illustrates the beauty of Fowler's Refactoring book, since he wrote it in a way that his solutions were reusable and the code smells simply reference them. However, for the sake of clarity and the ability for each article to be read as a standalone piece, I will likely reuse some prose and examples. I would also note that the problems the same refactor solves may be distinct, and understanding how the same pattern can apply to multiple code smells will strengthen your mental model and understanding of how to write "clean" code.
Links to other posts in this series:
Principles of Object-Oriented Programming
To frame this discussion of what violates the best practices of object-oriented programming (OOP) we first must understand the core principles that guide us.
Inheritance
Inheritance is the property that allows us to derive new classes by extending existing ones, thereby enabling code reuse. My favorite definition for inheritance comes from Deborah Armstrong's Quarks of Object Oriented Development (2006) as
a mechanism that allows the data and behavior of one class to be included in or used as the basis for another class
Consider an example where we have two classes representing animals:
class Dog:
def __init__(self, name):
self.name = name
def walk(self):
print("Walks on four legs")
def bark(self):
print("woof!")
class Cat:
def __init__(self, name):
self.name = name
def walk(self):
print("Walks on four legs")
def meow(self):
print("meow!")
We can simplify the above code by moving the shared properties into a parent class called Animal that Dog and Cat both inherit from:
class Animal:
def __init__(self, name):
self.name = name
def walk(self):
print("Walks on four legs")
class Dog(Animal):
def bark(self):
print("woof!")
class Cat(Animal):
def meow(self):
print("meow!")
This facilitates code reuse as we can easily create new animals, improves maintainability since changes to the Animal superclass only live in one location, and provides a built-in delegation mechanism such that subclasses can implement their own behaviors.
Encapsulation
The idea behind encapsulation is controlling what is accessible from outside an object. In other words, a clear division between an object's private internal state and its contract with (methods exposed to other objects/clients). In his 1986 paper Encapsulation and inheritance in object-oriented programming, Snyder posits that one should minimize the exposure of a module's implementation details to its clients to allow for changes made within the module without affecting the clients. This is particularly relevant to states within an object.
Imagine we introduce an energy state, indicating how energized the animal feels and some new methods that interact with it. Observe what happens when the state is left public (exposed in the interface):
class Animal:
def __init__(self, name):
self.name = name
self.energy = 100 # public — anyone can set it to anything
def walk(self):
if self.energy < 10:
print(f"{self.name} is too tired to walk")
return
self.energy -= 10
print(f"{self.name} walks on four legs")
def feed(self, amount):
self.energy = min(100, self.energy + amount)
cat = Cat("Whiskers")
cat.energy = 5000 # nonsense, but nothing prevents it
cat.walk() # energy is now 4990
cat.energy = -20 # the cat now has negative energy??
To prevent this from occurring we encapsulate energy (the Python convention is to prefix with _ ) so that it can only be accessed within the class. Then we create and expose special method for the public interface:
class Animal:
def __init__(self, name):
self.name = name
self._energy = 100
# ... walk and feed implementation
@property
def energy(self):
return self._energy
Note that the combination of the @property decorator with no setter is what truly blocks assignment– the leading underscore is just convention.
We can even change the derivation of energy within the class without affecting the way the client interacts with the object at all:
class Animal:
def __init__(self, name):
self.name = name
self._activity = 0
@property
def energy(self):
return max(0, 100 - self._activity) # now derived
Polymorphism
Subtype polymorphism is what enables a method to use a uniform interface regardless of the specific type. This is best illustrated with an example.
Consider a variation of the example we've been working with using a generic speak method that is concretely implemented by the subclasses:
class Animal:
def __init__(self, name):
self.name = name
def walk(self):
print("Walks on four legs")
def speak(self):
raise NotImplementedError
class Dog(Animal):
def speak(self):
print("woof!")
class Cat(Animal):
def speak(self):
print("meow!")
The speak method can now be called on any Animal. The client does not know or care about specific details as those are handled within the subclasses. This relates back to the Liskov substitution principle, which posits that an object of the superclass (Animal) can be replaced by an object of the subclass (Dog or Cat) without breaking the program.
Abstraction
Abstraction is the mechanism that ties everything together and allows us to provide simplified models (classes) for complex realities to suppress irrelevant details. In this regard, a class can be thought of as an abstraction of an object, such that the class encapsulates data and behavior, and inheritance is what allows the encapsulated data and behavior to be based on an existing class (Armstrong, 2006). Finally, polymorphism enables different objects to respond to the same invocations and respond appropriately.
Repeated Switches
This is another way of saying if/else blocks that appear in more than one location in the system. The primary concern is that changes or additions made to any one switch statement must occur in every instance of the repeated switch.
What makes this smell an Object-Orientation Abuser is that the repeated switch approach neglects the elegant solution of polymorphism offered by object-oriented design, in which we significantly improve the readability and organization of the code.
Refactoring Solutions
Replace Conditional with Polymorphism
This technique is actually quite similar to the Strategy Pattern (which I previously wrote about). The goal is to eliminate large conditional branches by replacing them with classes and polymorphism. Consider the logic we use to assign a dough, which involves adding a different flour for each distinct type:
def make_dough(dough_variant):
if variant == "regular":
add_flour()
if variant == "gluten-free":
add_almond_flour()
# New variant: whole wheat
if variant == "whole wheat":
add_whole_wheat_flour()
Instead of repeating the conditional or decomposing it, we instead convert it to it's own type, letting each concrete implementation handle the implementation details:
class Dough:
def add_flour(self):
# Implemented by specific types
class RegularDough(Dough):
def add_flour(self):
add_flour()
class GlutenFreeDough(Dough):
def add_flour(self):
add_almond_flour()
class WholeWheatDough(Dough):
def add_flour(self):
add_whole_wheat_flour()
def make_dough(dough):
activate_yeast()
add_olive_oil()
dough.add_flour()
# ... rest of steps
Now instead of adding a conditional branch whenever we need to add a new variant, we can simply add a child to the Dough class, then define the add_flour method.
Temporary Field
The temporary field describes a field that is only set sometimes, adding ambiguity to the object or function containing it. The solution involves a sequence of three strategies that build upon each other.
Consider the following example of a PizzaOrder class:
class PizzaOrder:
def __init__(self, items, is_delivery):
self.items = items
self.is_delivery = is_delivery
self.delivery_address = None
self.delivery_fee = None
self.driver_tip = None
def total(self):
subtotal = sum(item.price for item in self.items)
if self.is_delivery:
return subtotal + self.delivery_fee + self.driver_tip
return subtotal
The primary issue here is that three of the variables (delivery_address, delivery_fee, and driver_tip) are dead weight when the order is not a delivery.
Refactoring Solution
Extract Class
First, these inconsistent variables need a dedicated place to live, such as a dedicated Delivery class:
class Delivery:
def __init__(self, address, fee, tip):
self.address = address
self.fee = fee
self.tip = tip
Move Function
Next, the behavior related to those fields (computing the subtotal) is moved to a method within the new class:
class Delivery:
# ... init
def cost(self):
return self.fee + self.tip
This allows us to refactor the original PizzaOrder total method to the following:
class PizzaOrder(self):
def total(self):
subtotal = sum(item.price for item in self.items)
if self.delivery:
return subtotal + self.delivery.cost()
return subtotal
The combination of these two changes enforce the single-responsibility principle since the knowledge of the delivery cost lives with the delivery data instead of bleeding into the order, and improve encapsulation by exposing a single cost method to handle the implementation details.
Introduce Special Case
Fowler proposes one additional step, which is to create a new class to handle the conditional. Introduce Special Case involves creating a class to capture common behaviors to replace special-case checks, and is commonly referred to as the Null Object pattern because null is frequently the value involved in special-case processing (Fowler, p.289). In the PizzaOrder scenario, we would introduce a special class NoDelivery to handle the case when delivery is null:
class NoDelivery: # the special case
def cost(self):
return 0
Then, instead of our guard checking if self.delivery is truthy, we can replace the variable assignment with self.delivery = delivery or NoDelivery(), which will guarantee that self.delivery.cost() is always callable.
Alternative Classes with Different Interfaces
This smell involves two or more classes that essentially do the same job but have different method names or signatures, thereby preventing client code from using them interchangeably despite being conceptually the same. Take the following example where two similar classes are used to notify customers that their pizza is on the way using a notify_customer method:
class EmailNotifier:
def send_email(self, address, subject, body):
print(f"Emailing {address}: {subject}")
class SmsNotifier:
def dispatch_text(self, phone_number, message):
print(f"Texting {phone_number}: {message}")
def notify_customer(notifier, customer):
if isinstance(notifier, EmailNotifier):
notifier.send_email(customer.email, "Pizza Update", "On its way!")
elif isinstance(notifier, SmsNotifier):
notifier.dispatch_text(customer.phone, "Pizza on its way!")
Refactoring Solutions
Change Function Declaration
This refactoring technique encompasses both renaming the method itself and modifying the parameters (Fowler, p.125).
The first step is to align the method names (we'll use notify). Next, look at both the declarations and match up the signatures:
class EmailNotifier:
def notify(self, recipient, message):
print(f"Emailing {recipient}: {message}")
class SmsNotifier:
def notify(self, recipient, message):
print(f"Texting {recipient}: {message}")
Now that both classes contain the same general interface, the client no longer cares which type it receives– it can invoke them interchangeably!
def notify_customer(notifier, customer):
notifier.notify(customer.contact, "Pizza on its way!")
Move Function
Move function comes into play when the alternative classes have somewhat different capabilities (but the core functionality remains the same). For example, let's say the EmailNotifier implementation had a record_sent method to log each notification to the company's audit log:
class EmailNotifier:
def notify(self, recipient, message):
print(f"Emailing {recipient}: {message}")
self.record_sent(recipient, message)
def record_sent(self, recipient, message):
audit_log.append((recipient, message))
class SmsNotifier:
def notify(self, recipient, message):
print(f"Texting {recipient}: {message}")
# no logging here — the caller has to do it
def notify_customer(notifier, customer):
notifier.notify(customer.contact, "Pizza on its way!")
if isinstance(notifier, SmsNotifier):
audit_log.append((customer.contact, "Pizza on its way!"))
We can move the logging behavior into the SmsNotifier to ensure consistency between the classes, keeping the client code lean:
class EmailNotifier:
def notify(self, recipient, message):
print(f"Emailing {recipient}: {message}")
self.record_sent(recipient, message)
def record_sent(self, recipient, message):
audit_log.append((recipient, message))
class SmsNotifier:
def notify(self, recipient, message):
print(f"Texting {recipient}: {message}")
self.record_sent(recipient, message)
def record_sent(self, recipient, message):
audit_log.append((recipient, message))
def notify_customer(notifier, customer):
notifier.notify(customer.contact, "Pizza on its way!")
Extract Superclass
While not necessary, taking this a step further to make the code more extensible in the future might involve pulling the shared notify interface into one Notifier superclass so that additional notifiers can be added in the future.
Refused Bequest
Refused Bequest describes a two ways in which a subclass can violate the inheritance contract (Fowler, p.84):
- Subclass only uses some of the methods and/or fields inherited from its parent.
class Enemy:
def __init__(self, name, hp):
self.name = name
self.hp = hp
def take_damage(self, amount):
self.hp -= amount
def move(self):
print(f"{self.name} moves toward you")
def flee(self):
print(f"{self.name} runs away")
class Boss(Enemy):
def take_damage(self, amount):
self.hp -= amount
def move(self):
print(f"{self.name} moves toward you")
# a Boss never flees, so it just ignores the inherited flee()
- Subclass that reuses behavior but fails to support the interface of the superclass. This occurs when the subclass inherits a method that it cannot actually fulfill (meaning it throws an error or returns something nonsensical).
class Enemy:
# ... init and take_damage from before
def move(self):
print(f"{self.name} moves toward you")
def flee(self):
print(f"{self.name} runs away")
class Turret(Enemy):
def move(self):
raise NotImplementedError("turrets can't move")
def flee(self):
raise NotImplementedError("turrets can't flee")
While Fowler considers the former case a trivial issue often not worth cleaning up, he emphasizes the severity of the latter violation. This smell also represents a violation of the Liskov Substitution Principle and the promise of inheritance as we can no longer guarantee that any subclass of Enemy can be substitued without breaking the program.
Refactoring Solutions
Push Down Method / Push Down Field
To resolve the tension of the mild case, the solution involves moving the unused methods and fields out of the parent and down into the siblings that actually use them. Say, taking flee out of Enemy and wiring it into the subclasses that actually use it, preventing the Boss subclass from ever receiving it.
Replace Subclass with Delegate
Rather than break the inheritance contract, the fix is to hold a reference to the superclass, then extract the parts it wants.
class Turret:
def __init__(self, name, hp):
self._enemy = Enemy(name, hp)
def take_damage(self, amount):
self._enemy.take_damage(amount)
def attack(self):
print(f"{self._enemy.name} fires from a fixed position")
# omitted move() and flee()
The tradeoff here is that we are forwarding the boilerplate code (take_damage and any other methods we delegate) from the Enemy class in order to achieve a valid contract.
Replace Superclass with Delegate
This mis-inheritance also crops up on the parent's side when we drag an entire interface into a subclass only to use a portion of it. Fowler provides the example of making a stack a subclass of list, causing all of the list operations to be forwarded to stack despite most not applying. The solution is to make a new stack class with the list a property that we can delegate operations to (Fowler, p.399). Continuing with the video game example, imagine we have a Sprite class that we use for rendering drawings. We might logically attempt to make Enemy a subclass of Sprite in order to inherit the draw method:
class Sprite:
def __init__(self, image):
self.image = image
self.x = 0
self.y = 0
def draw(self):
print(f"drawing {self.image} at ({self.x}, {self.y})")
def set_position(self, x, y):
self.x, self.y = x, y
class Enemy(Sprite): # inherits Sprite to reuse drawing
def __init__(self, name, hp, image):
super().__init__(image)
self.name = name
self.hp = hp
The issue is that an Enemy is not truly a kind of Sprite, it simply uses one to render on the screen. Instead, we let Enemy hold a reference to a Sprite, delegating the behavior we need (with the caveat of forwarding boilerplate from before):
class Enemy:
def __init__(self, name, hp, image):
self.name = name
self.hp = hp
self._sprite = Sprite(image)
def draw(self):
self._sprite.draw() # delegate the rendering
def move_to(self, x, y):
self._sprite.set_position(x, y)
Final Thoughts
This category of code smells emphasizes how the core principles of OOP (inheritance, encapsulation, polymorphism, abstraction) operate in tandem to reduce waste and enforce a robust, maintainable codebase. One thing that I took away is the utility of Extract Class, not only in how it addresses encapsulation and the single-responsibility principle, but how it facilitates a shift in how I view code. I can now recognize when I'm "cross-contaminating" or overloading a class or function, and can more proactively refactor before it becomes a significant lift.
Additionally, the Refused Bequest smell got me thinking about contracts between objects in a way I hadn't considered before. While you can write code that compiles and runs despite violating an inheritance contract, it plants seeds for a future errors and forces the developer (or future reader) to perform more work trying to unravel why something broke. Further, the contract that is being broken is one that the compiler cannot see. While the compiler can enforce a method signature (e.g., invalid types, wrong number of arguments), the behavioral contracts (what a method promises to do) are invisible to it. Indeed, this is dangerous because it defers the cost and separates the failure point from the location of the code.
References:
Fowler, M. (2018). Refactoring: Improving the design of existing code (2nd ed.). Addison-Wesley Professional
Mäntylä, M.V., Lassenius, C. Subjective evaluation of software evolvability using code smells: An empirical study. Empir Software Eng 11, 395–431 (2006). https://doi.org/10.1007/s10664-006-9002-8
Black, A. Object-oriented programming: Some history, and challenges for the next fifty years. Information and Computation 231, 3-20 (2013). https://doi.org/10.1016/j.ic.2013.08.002
Snyder, A. Encapsulation and Inheritance in Object-Oriented Programming Languages. (1986). https://dl.acm.org/doi/10.1145/960112.28702
Armstrong, D. The Quarks of Object-Oriented Development. (2006). https://doi.org/10.1145/1113034.1113040
