Code Review Ep. 0: Violating Single Responsibility Principle

We’re starting a new series of blog posts: Code Review. Welcome to this very first blog post where we will conduct a code review of the violation of Single Responsibility Principle in Swift.

We’ll analyze the code to identify areas that violate the Single Responsibility Principle (SRP) and discuss potential improvements to enhance the code’s structure and maintainability.

Code Review: Analyzing the Order Class

class Order {
    var items: [String]
    var totalPrice: Double
    
    init(items: [String]) {
        self.items = items
        self.totalPrice = 0.0
    }
    
    func calculateTotalPrice() {
        for item in items {
            totalPrice += getItemPrice(item)
        }
    }
    
    func getItemPrice(_ item: String) -> Double {
        let price = 10.0
        return price
    }
}

Upon reviewing the code, we’ve identified a few areas where the Order class violates the SRP, which emphasizes maintaining a single responsibility per class. Let’s delve into these areas and propose potential solutions for improvement.

“Software engineering is the art of creating robust and elegant solutions that empower us to turn complex problems into manageable code.”

0. Responsibility Mixing:

The Order class currently handles both representing an order and calculating the price of each item. This mixture of responsibilities can lead to code that is difficult to comprehend and maintain. To address this, we need to separate these distinct responsibilities into separate classes or functions.

Suggested Solution:

• Create a dedicated class or function responsible for calculating item prices.
• Move the logic for retrieving and calculating item prices out of the Order class.

Sample Solution:

class PriceCalculator {
    func getItemPrice(_ item: String) -> Double {
        // Logic to fetch the price of the item from a database or external service
        let price = 10.0 // Placeholder value
        return price
    }
}

1. calculateTotalPrice() Method:

The calculateTotalPrice() method, although related to an order, is better suited to a separate price calculation component. This segregation of responsibilities will help maintain a clear separation of concerns within the codebase.

Suggested Solution:

• Extract the logic for calculating the total price into a separate class or function that is solely responsible for price calculations.
• The Order class should delegate the task of calculating the total price to this dedicated component.

Sample Solution:

class PriceCalculator {
    func calculateTotalPrice(for items: [String]) -> Double {
        var totalPrice = 0.0
        for item in items {
            let price = getItemPrice(item)
            totalPrice += price
        }
        return totalPrice
    }
}

class Order {
    var items: [String]
    
    init(items: [String]) {
        self.items = items
    }
}

2. getItemPrice() Method:

The getItemPrice() method within the Order class is responsible for both retrieving and calculating item prices. This violates the SRP, as the class should solely focus on representing the order, not performing calculations.

Suggested Solution:

• Create a separate class or function responsible for retrieving and calculating item prices.
• Move the logic for retrieving the price of an item into this dedicated component.

Sample Solution:

class PriceRetriever {
    func getItemPrice(_ item: String) -> Double {
        // Logic to fetch the price of the item from a database or external service
        let price = 10.0 // Placeholder value
        return price
    }
}

class Order {
    var items: [String]
    var priceRetriever: PriceRetriever
    
    init(items: [String], priceRetriever: PriceRetriever) {
        self.items = items
        self.priceRetriever = priceRetriever
    }
    
    func calculateTotalPrice() -> Double {
        var totalPrice = 0.0
        for item in items {
            let price = priceRetriever.getItemPrice(item)
            totalPrice += price
        }
        return totalPrice
    }
}

Conclusion:

During our code review of the Order class, we identified several areas that violated the Single Responsibility Principle (SRP). By separating concerns, creating dedicated classes or functions, and delegating responsibilities, we can improve the code’s structure and maintainability. Following principles like SRP enhances the quality of code, making it easier to understand, modify, and collaborate on projects.

SRP stands for Single Responsibility Principle. It is a software development principle that states that a class or module should have only one reason to change. In other words, it should have a single responsibility or purpose. By adhering to SRP, we aim to create code that is modular, maintainable, and easier to understand.