Lesson 1
Refactoring Code Smells in C++
Topic Overview

Welcome to the world of refactoring! We're learning about Code Smells, which are patterns in code that hint at potential problems. Our mission is to help you spot these smells and understand how to improve them through refactoring. We'll delve into the concept of Code Smell, examine different types, and apply real-world code examples to solidify your understanding. Let's get started!

Introduction to Code Smells

Code smells indicate something might be off within your code, akin to a warning that the code may not be as readable, efficient, or manageable as it could be.

Consider this bit of code:

C++
1double calculate(int quantity, double price) { 2 return quantity * price; 3} 4 5double total = calculate(5, 3.0);

The function name calculate is too vague. What exactly does it calculate? For whom? This ambiguity is a sign of a 'bad naming' code smell.

Duplicate Code

If you notice the same piece of code in more than one place, you may be looking at an example of the Duplicate Code smell. Duplicate code leaves room for errors and bugs. If you need to make a change, you might overlook one instance of duplication.

Here's a simple example:

C++
1double total_apples_price = quantity_apples * price_apple - 5; 2double total_bananas_price = quantity_bananas * price_banana - 5;

This code performs the same operation on different data. Instead of duplicating the operation, we can create a function to handle it:

C++
1double calculate_price(int quantity, double price) { 2 double discount = 5.0; 3 return quantity * price - discount; 4} 5 6double total_apples_price = calculate_price(quantity_apples, price_apple); 7double total_bananas_price = calculate_price(quantity_bananas, price_banana);

With this solution, if we need to change the discount or the formula, we can do so in one place: the calculate_price function.

Too Long Method

A method that does too many things or is too long is harder to read and understand, making it a prime candidate for the Too Long Method smell.

Consider this example:

C++
1class Order { 2public: 3 std::string payment_type; 4 bool is_valid() const { /* validation logic */ return true; } 5 // Other order-related methods and attributes 6}; 7 8bool process_order(Order& order) { 9 std::cout << "Processing order..." << std::endl; 10 if (order.is_valid()) { 11 std::cout << "Order is valid" << std::endl; 12 if (order.payment_type == "credit_card") { 13 process_credit_card_payment(order); 14 send_order_confirmation_email(order); 15 } else if (order.payment_type == "paypal") { 16 process_paypal_payment(order); 17 send_order_confirmation_email(order); 18 } else if (order.payment_type == "bank_transfer") { 19 process_bank_transfer_payment(order); 20 send_order_confirmation_email(order); 21 } else { 22 std::cout << "Unsupported payment type" << std::endl; 23 return false; 24 } 25 std::cout << "Order processed successfully!" << std::endl; 26 return true; 27 } else { 28 std::cout << "Invalid order" << std::endl; 29 return false; 30 } 31}

This function handles too many aspects of order processing, suggesting a 'Too Long Method' smell. A better approach could involve breaking down the functionality into smaller, more focused methods and using a cleaner structure for handling payment types, avoiding strings altogether.

Here's the refactored version:

C++
1enum class PaymentType { 2 CreditCard, 3 PayPal, 4 BankTransfer, 5 Unsupported 6}; 7 8class Order { 9public: 10 PaymentType payment_type; 11 bool is_valid() const { /* validation logic */ return true; } 12 // Other order-related methods and attributes 13}; 14 15bool process_payment(PaymentType payment_type, Order& order) { 16 switch (payment_type) { 17 case PaymentType::CreditCard: 18 process_credit_card_payment(order); 19 break; 20 case PaymentType::PayPal: 21 process_paypal_payment(order); 22 break; 23 case PaymentType::BankTransfer: 24 process_bank_transfer_payment(order); 25 break; 26 default: 27 std::cout << "Unsupported payment type" << std::endl; 28 return false; 29 } 30 return true; 31} 32 33bool process_order(Order& order) { 34 std::cout << "Processing order..." << std::endl; 35 if (!order.is_valid()) { 36 std::cout << "Invalid order" << std::endl; 37 return false; 38 } 39 40 if (process_payment(order.payment_type, order)) { 41 send_order_confirmation_email(order); 42 std::cout << "Order processed successfully!" << std::endl; 43 return true; 44 } else { 45 return false; 46 } 47}

The original process_order function was overly long, handling order validation, payment processing, and email sending within a single block. In the refactored version, functionality is divided into smaller functions, such as process_payment, which uses the PaymentType enum and a switch-case structure. This separation clarifies responsibilities and simplifies the logic, transforming the code into more readable, manageable units. The resulting code is more modular, easier to maintain, and focused on single responsibilities, effectively addressing the initial 'Too Long Method' smell.

Comment Abuse

Comments within your code should provide useful information, but remember, too much of a good thing can be a problem. Over-commenting can distract from the code itself and might indicate unclear code.

Consider this revised function, which calculates the area of a triangle, now with comments:

C++
1double calculate_triangle_area(double base, double height) { 2 // Calculate the area of a triangle 3 // Formula: 0.5 * base * height 4 double area = 0.5 * base * height; // Area calculation 5 return area; // Return result 6}

While comments explaining the formula might be helpful for some, the code itself is quite straightforward, and the comments on the calculation itself might be seen as unnecessary. If the function's name and parameters are clear, the need for additional comments can be minimized.

Bad Naming

Finally, we have Bad Naming. As the name suggests, this smell occurs when names don't adequately explain what a variable, function, or class does. Good names are crucial for readable, understandable code.

Take a look at the following example:

C++
1int func(int a, int b) { 2 return a * 10 + b; 3}

The names func, a, and b don't tell us much about what is happening. A better version could be this:

C++
1int calculate_score(int base_score, int extra_points) { 2 return base_score * 10 + extra_points; 3}

In this version, each name describes the data or action it represents, making the code easier to read.

Lesson Summary and Practice

We've discovered Code Smells and studied common types: Duplicate Code, Too Long Method, Comment Abuse, and Bad Naming. Now you can spot code smells and understand how they can signal a problem in your code.

In the upcoming real-world, example-based practice sessions, you'll enhance your debugging skills and improve your code's efficiency, readability, and maintainability. How exciting is that? Let's move ahead!

Enjoy this lesson? Now it's time to practice with Cosmo!
Practice is how you turn knowledge into actual skills.