Refactoring and Code Smells in Ruby

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

Code smells are signs that something could be amiss in our code. You could compare them to an unpleasant smell in a room. But instead of indicating rotten food or a dirty sock, they signal that our code may not be as readable, efficient, or manageable as it could be.

Consider this bit of code:

def calculate(quantity, price)
  quantity * price
end

total = calculate(5, 3)

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

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 an example:

total_apples_price = quantity_apples * price_apple - 5
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 method to handle it:

def calculate_price(quantity, price)
  discount = 5
  quantity * price - discount
end

total_apples_price = calculate_price(quantity_apples, price_apple)
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 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:

def process_order(order)
  puts "Processing order..."
  if order.valid?
    puts "Order is valid"
    if order.payment_type == "credit_card"
      process_credit_card_payment(order)
      send_order_confirmation_email(order)
    elsif order.payment_type == "paypal"
      process_paypal_payment(order)
      send_order_confirmation_email(order)
    elsif order.payment_type == "bank_transfer"
      process_bank_transfer_payment(order)
      send_order_confirmation_email(order)
    else
      puts "Unsupported payment type"
      return false
    end
    puts "Order processed successfully!"
    true
  else
    puts "Invalid order"
    false
  end
end

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.

For example, the updated code can look like this:

def process_payment(payment_type, order)
  case payment_type
  when "credit_card"
    process_credit_card_payment(order)
  when "paypal"
    process_paypal_payment(order)
  when "bank_transfer"
    process_bank_transfer_payment(order)
  else
    puts "Unsupported payment type"
    return false
  end
  true
end

def process_order(order)
  puts "Processing order..."
  unless order.valid?
    puts "Invalid order"
    return false
  end
  
  if process_payment(order.payment_type, order)
    send_order_confirmation_email(order)
    puts "Order processed successfully!"
    true
  else
    false
  end
end