Breaking Up with Bad Design: Recognizing Object-Orientation Abusers in Your Code and How to Fix Them — Story 1
Object-Orientation Abusers are incomplete or incorrect application of object-oriented programming principles, resulting in code that is difficult to maintain, understand, and extend. This can cause because of following reasons —
- Switch Statements
- Temporary Field
- Refused Bequest
- Alternative classes with Different interfaces
Sometimes Frankenstein’s Monster Object (not be a cohesive or well-designed solution) tends to be violating the Single-Responsibility Principle.
Switch Statements
You have a complex switch operator or sequence of if statements.
🚧 Problem 🚧 → To isolate switch statements — Extract or Move Method
// Problem of Object-Orientation Abusers with switch statements
public String getBalanceStatus() {
if (balance >= 1000) {
return "You have a lot of money!";
} else if (balance >= 100) {
return "You have some money.";
} else {
return "You have no money.";
}
}This is problematic because it violates the Open-Closed Principle. If the criteria for determining the message need to change in the future, such as adding a new message for balance over $10,000, the switch statement would need to be modified. This violates the principle of being closed for modification and can lead to errors if the modification is not done correctly.
💡Treatment 💡
public String getBalanceStatus() {
switch(getBalanceCategory()) {
case "high":
return "You have a lot of money!";
case "medium":
return "You have some money.";
case "low":
return "You have no money.";
default:
return "Error: balance category not recognized.";
}
}
private String getBalanceCategory() {
if (balance >= 1000) {
return "high";
} else if (balance >= 100) {
return "medium";
} else {
return "low";
}
}🚧 Problem 🚧 → if a switch statement is based on a type code, consider using Replace Type Code with State, or Replace Type Code with Enum Class.
fun processShape(shape: Shape, mode: Int) {
when (mode) {
1 -> if (shape is Circle) {
// do something specific to Circle
} else if (shape is Rectangle) {
// do something specific to Rectangle
} else if (shape is Triangle) {
// do something specific to Triangle
}
2 -> if (shape is Circle) {
// do something different for Circle
} else if (shape is Rectangle) {
// do something different for Rectangle
} else if (shape is Triangle) {
// do something different for Triangle
}
else -> throw IllegalArgumentException("Invalid mode: $mode")
}
}Above code violates the principles of object-oriented design by relying on a switch statement to determine behavior based on the type of an object. This approach creates tight coupling between the calling code and the implementation details of the Shape hierarchy, and can make it difficult to add new shapes or behaviors in the future.
💡Treatment 💡 → “Replace Type Code with Subclass” treatment (abstract).
abstract class Shape {
abstract fun process(mode: Int)
}
class Circle : Shape() {
override fun process(mode: Int) {
// do something specific to Circle
}
}
class Rectangle : Shape() {
override fun process(mode: Int) {
// do something specific to Rectangle
}
}
class Triangle : Shape() {
override fun process(mode: Int) {
// do something specific to Triangle
}
}
fun processShape(shape: Shape, mode: Int) {
shape.process(mode)
}💡Treatment💡 → Treatment with State/Strategy (interface)
interface ShapeBehavior {
fun process()
}
class CircleBehavior : ShapeBehavior {
override fun process() {
// do something specific to Circle
}
}
class RectangleBehavior : ShapeBehavior {
override fun process() {
// do something specific to Rectangle
}
}
class TriangleBehavior : ShapeBehavior {
override fun process() {
// do something specific to Triangle
}
}
class Shape(private val behavior: ShapeBehavior) {
fun process() {
behavior.process()
}
}
fun processShape(shape: Shape) {
shape.process()
}💡Treatment💡 → Treatment with enum class
enum class Shape {
CIRCLE {
override fun process(mode: Int) {
// do something specific to Circle
}
},
RECTANGLE {
override fun process(mode: Int) {
// do something specific to Rectangle
}
},
TRIANGLE {
override fun process(mode: Int) {
// do something specific to Triangle
}
};
abstract fun process(mode: Int)
}
fun processShape(shape: Shape, mode: Int) {
shape.process(mode)
}🚧 Problem 🚧 → After specifying the inheritance structure, use the Replace Conditional with Polymorphism technique.
class Shape(val area: Double){
fun getArea(): Double {
return area
}
}
class Rectangle(val width: Double, val height: Double) : Shape(width * height)
class Circle(val radius: Double) : Shape(Math.PI * radius * radius)
fun main() {
val shapes = listOf( Rectangle(5.0, 10.0), Circle(3.0) )
for (shape in shapes) {
if (shape is Rectangle) {
println("Rectangle area: ${shape.width * shape.height}")
} else if (shape is Circle) {
println("Circle area: ${Math.PI * shape.radius * shape.radius}")
}
}
}It violates the principles of object-oriented programming, which promotes encapsulating behavior within objects and using polymorphism to delegate behavior to the appropriate object. Instead of using a conditional statement to determine the behavior of the program, the behavior should be encapsulated within each object.
In this case, the Rectangle and Circle classes both inherit from the Shape class and have their own specific implementations of the getArea method. Therefore, instead of using a conditional statement to determine the behavior based on the type of each shape, we should simply call the getArea method on each shape object.
By doing so, we delegate the responsibility of computing the area to the appropriate object and avoid the conditional statement that checks the type of each object. This approach adheres to the principles of polymorphism, and it’s more extensible, maintainable, and easier to reason about.
💡Treatment💡 → use Replace Conditional with Polymorphism technique
class Shape{
abstract fun getShape(): Double
}
class Rectangle(val width: Double, val height: Double): Shape{
override fun getShape(): Double = width * height
}
class Circle(val radius: Double): Shape{
override fun getShape(): Double = Math.PI * radius * radius
}
fun main() {
val shapes = listOf( Rectangle(5.0, 10.0), Circle(3.0) )
for (shape in shapes) {
println("Shape area: ${shape.getArea()}")
}
}🚧 Problem 🚧 → Simplify operator conditions by using Replace Parameter with Explicit Method to avoid violating the Single Responsibility Principle.
fun doSomething(condition: String){
when(condition) {
"A" -> callMethod("paramA")
"B" -> callMethod("paramB")
"C" -> callMethod("paramC")
else -> callMethod("default")
}
}The problem with this code is that it violates the Single Responsibility Principle. The doSomething function is responsible for both choosing the appropriate condition and calling the callMethod function with a specific parameter. This approach leads to coupling between the doSomething function and the callMethod function, which can make the code harder to maintain and extend over time.
💡Treatment💡
fun doSomething(condition: String) {
when(condition) {
"A" -> doSomethingA()
"B" -> doSomethingB()
"C" -> doSomethingC()
else -> doDefault()
}
}
fun doSomethingA() {
callMethod("paramA")
fun doSomethingB() {
callMethod("paramB")
}
fun doSomethingC() {
callMethod("paramC")
}
fun doDefault() {
callMethod("default")
}Temporary Field
Use Extract Class to move temporary fields and their code into a different class, as an alternative to Replace Method with Method Object.
class Order {
var items: List<Item> = listOf()
fun calculateTotalPrice(): Double {
var totalPrice = 0.0
for (item in items) {
totalPrice += item.price
}
// Perform additional calculations using totalPrice
// ...
return totalPrice
}
fun printOrderDetails() {
val orderNumber = 12345
val orderDate = LocalDate.now()
println("Order Number: $orderNumber")
println("Order Date: $orderDate")
// Perform additional print operations using orderNumber and orderDate
// ...
}
}
class Item(val name: String, val price: Double)The totalPrice field is temporary and used to calculate the total price of an order.
Similarly, orderNumber and orderDate fields are temporary and used to print the details of an order.
💡Treatment💡
class OrderPriceCalculator(
private val items: List<Item>
){
fun calculateTotalPrice(): Double = items.sumOf { it.price }
}
class OrderDetailPrinter(
private val orderNumber: Int,
private val orderDate: LocalDate
){
fun printOrderDetail() {
println("Order Number: $orderNumber")
println("Order Date: $orderDate")
}
}
class Order constructor(
private val items: List<Item>
) {
fun calculateTotalPrice(): Double =
OrderPriceCalculator(items).calculateTotalPrice()
fun printOrderDetail(){
val (orderNumber, orderDate) = 12345 to LocalDate.now()
OrderDetailPrinter(orderNumber,orderDate).printOrderDetail()
}
}
class Item(val name: String, val price: Double)🚧 Problem 🚧 → Replace the conditional check for temporary field values with a Null Object (Introduce Null Object or Null Object Pattern)
class User(private val name: String, private val email: String?) {
fun getEmail(): String {
return email ?: throw IllegalStateException("Email is null!")
}
}
// Post.kt
class Post(private val title: String, private val author: User) {
fun getAuthorEmail(): String {
return author.getEmail()
}
}The Userclass has two fields:nameand email, where emailis nullable.
The Postclass has two fields: titleand author, where author is an instance of User .
💡Treatment💡
// User.kt
object NullUser : User("NULL", "", "")
class User(private val name: String, private val email: String?) {
fun getEmail(): String {
return email ?: NullUser.getEmail()
}
}
// Post.kt
object NullPost : Post("NULL", NullUser())
class Post(private val title: String, private val author: User) {
fun getAuthorEmail(): String {
return author.getEmail() ?: NullPost.author..getEmail()
}
}I will explain Refused Bequest and Alternative classes with Different interfaces in next Story …..
See you next time, bye-bye 👋
