Refactoring is the process of improving code structure without changing its functionality. Here are practical refactoring examples that will help you recognize code smells and transform them into clean, maintainable Rails code.

Controller Refactoring

1. Fat Controller → Skinny Controller with Service Objects

Before (Fat Controller):
```ruby
class OrdersController < ApplicationController
def create
@order = Order.new(orderparams)
@order.user = currentuser
@order.status = 'pending'
@order.ordernumber = generateorder_number

if @order.save
# Calculate total with tax and shipping
subtotal = @order.items.sum { |item| item.price * item.quantity }
tax = subtotal * 0.08
shipping = subtotal > 50 ? 0 : 10
@order.update(
subtotal: subtotal,
tax: tax,
shipping: shipping,
total: subtotal + tax + shipping
)

# Update inventory
@order.items.each do |item|
product = item.product
product.update(stock: product.stock - item.quantity)

# Send low stock alert
if product.stock < 5
AdminMailer.lowstockalert(product).deliver_now
end
end

# Send confirmation email
OrderMailer.confirmation(@order).deliver_now

# Create loyalty points
points = (@order.total * 0.1).round
currentuser.loyaltypoints += points
current_user.save

redirect_to @order, notice: 'Order created successfully!'
else
render :new
end
end

private

def generateordernumber
"ORD-#{Time.current.strftime('%Y%m%d')}-#{SecureRandom.hex(4).upcase}"
end
end
```

After (Skinny Controller with Service Object):
```ruby

app/controllers/orders_controller.rb

class OrdersController < ApplicationController
def create
service = OrderCreationService.new(currentuser, orderparams)

if service.call
redirect_to service.order, notice: 'Order created successfully!'
else
@order = service.order
render :new
end
end
end

app/services/ordercreationservice.rb

class OrderCreationService
attr_reader :order, :errors

def initialize(user, orderparams)
@user = user
@orderparams = order_params
@errors = []
end

def call
ActiveRecord::Base.transaction do
createorder
calculatetotals
updateinventory
awardloyaltypoints
sendconfirmation_email
end

order.persisted? && errors.empty?
rescue => e
@errors << e.message
false
end

private

attrreader :user, :orderparams

def createorder
@order = Order.create!(
orderparams.merge(
user: user,
status: 'pending',
order_number: OrderNumberGenerator.generate
)
)
end

def calculate_totals
calculator = OrderTotalCalculator.new(@order)
calculator.calculate!
end

def update_inventory
InventoryUpdateService.new(@order).call
end

def awardloyaltypoints
LoyaltyPointsService.new(user, @order.total).award_points
end

def sendconfirmationemail
OrderMailer.confirmation(@order).deliver_later
end
end
```

2. Complex Conditional Logic → Strategy Pattern

Before:
```ruby
class PaymentController < ApplicationController
def process
case params[:paymentmethod]
when 'creditcard'
if params[:cardnumber].length == 16
charge = Stripe::Charge.create(
amount: (params[:amount].tof * 100).to_i,
currency: 'usd',
source: params[:token]
)

if charge.paid
Payment.create!(
amount: params[:amount],
method: 'creditcard',
externalid: charge.id,
status: 'completed'
)
redirectto successpath
else
flash[:error] = 'Payment failed'
redirectto paymentpath
end
else
flash[:error] = 'Invalid card number'
redirectto paymentpath
end

when 'paypal'
# PayPal processing logic...
response = PayPal::SDK::REST::Payment.new({
intent: 'sale',
payer: { payment_method: 'paypal' },
# ... complex PayPal setup
})

if response.create
# Handle success
else
# Handle failure
end

when 'bank_transfer'
# Bank transfer logic...
end
end
end
```

After:
```ruby

app/controllers/payment_controller.rb

class PaymentController < ApplicationController
def process
processor = PaymentProcessorFactory.create(
params[:paymentmethod],
paymentparams
)

result = processor.process

if result.success?
redirectto successpath, notice: 'Payment successful!'
else
flash[:error] = result.errormessage
redirectto payment_path
end
end
end

app/services/paymentprocessorfactory.rb

class PaymentProcessorFactory
def self.create(method, params)
case method
when 'creditcard'
CreditCardProcessor.new(params)
when 'paypal'
PaypalProcessor.new(params)
when 'banktransfer'
BankTransferProcessor.new(params)
else
raise ArgumentError, "Unknown payment method: #{method}"
end
end
end

app/services/creditcardprocessor.rb

class CreditCardProcessor
def initialize(params)
@params = params
end

def process
return failureresult('Invalid card number') unless validcard?

charge = createstripecharge

if charge.paid
createpaymentrecord(charge)
successresult
else
failureresult('Payment failed')
end
rescue Stripe::CardError => e
failure_result(e.message)
end

private

def validcard?
@params[:cardnumber]&.length == 16
end

def createstripecharge
Stripe::Charge.create(
amount: (@params[:amount].tof * 100).toi,
currency: 'usd',
source: @params[:token]
)
end

def createpaymentrecord(charge)
Payment.create!(
amount: @params[:amount],
method: 'creditcard',
externalid: charge.id,
status: 'completed'
)
end

def success_result
PaymentResult.new(success: true)
end

def failureresult(message)
PaymentResult.new(success: false, errormessage: message)
end
end

app/models/payment_result.rb

class PaymentResult
attrreader :errormessage

def initialize(success:, errormessage: nil)
@success = success
@errormessage = error_message
end

def success?
@success
end
end
```

Model Refactoring

3. Fat Model → Skinny Model with Concerns

Before:
```ruby
class User < ApplicationRecord
hasmany :posts
hasmany :comments

# Authentication methods
def authenticate(password)
BCrypt::Password.new(password_digest) == password
end

def generatepasswordresettoken
self.passwordresettoken = SecureRandom.urlsafebase64
self.passwordresetsent_at = Time.current
save!
end

def passwordresetexpired?
passwordresetsent_at < 2.hours.ago
end

# Avatar methods
def avatarurl(size = :medium)
if avatar.present?
avatar.variant(resizetolimit: [avatarsize(size), nil])
else
defaultavatarurl(size)
end
end

def avatar_size(size)
case size
when :small then 50
when :medium then 100
when :large then 200
end
end

def defaultavatarurl(size)
"https://via.placeholder.com/#{avatar_size(size)}"
end

# Activity tracking
def tracklogin
update(lastloginat: Time.current, logincount: login_count + 1)
end

def activeuser?
lastlogin_at > 30.days.ago
end

def totalpoststhismonth
posts.where('createdat > ?', 1.month.ago).count
end

# Notification methods
def notify(message, type = :info)
notifications.create!(
message: message,
notification_type: type,
read: false
)
end

def unreadnotificationscount
notifications.where(read: false).count
end

def markallnotificationsread
notifications.where(read: false).updateall(read: true)
end
end
```

After:
```ruby

app/models/user.rb

class User < ApplicationRecord
include Authenticatable
include Avatareable
include Trackable
include Notifiable

hasmany :posts
hasmany :comments
end

app/models/concerns/authenticatable.rb

module Authenticatable
extend ActiveSupport::Concern

def authenticate(password)
BCrypt::Password.new(password_digest) == password
end

def generatepasswordresettoken
self.passwordresettoken = SecureRandom.urlsafebase64
self.passwordresetsent_at = Time.current
save!
end

def passwordresetexpired?
passwordresetsent_at < 2.hours.ago
end
end

app/models/concerns/avatareable.rb

module Avatareable
extend ActiveSupport::Concern

def avatarurl(size = :medium)
if avatar.present?
avatar.variant(resizetolimit: [avatarsize(size), nil])
else
defaultavatarurl(size)
end
end

private

def avatar_size(size)
{ small: 50, medium: 100, large: 200 }[size] || 100
end

def defaultavatarurl(size)
"https://via.placeholder.com/#{avatar_size(size)}"
end
end

app/models/concerns/trackable.rb

module Trackable
extend ActiveSupport::Concern

def tracklogin
update(lastloginat: Time.current, logincount: login_count + 1)
end

def activeuser?
lastlogin_at > 30.days.ago
end

def totalpoststhismonth
posts.where('createdat > ?', 1.month.ago).count
end
end

app/models/concerns/notifiable.rb

module Notifiable
extend ActiveSupport::Concern

included do
has_many :notifications, dependent: :destroy
end

def notify(message, type = :info)
notifications.create!(
message: message,
notification_type: type,
read: false
)
end

def unreadnotificationscount
notifications.where(read: false).count
end

def markallnotificationsread
notifications.where(read: false).updateall(read: true)
end
end
```

4. N+1 Query Problem → Eager Loading and Query Objects

Before:
```ruby
class PostsController < ApplicationController
def index
@posts = Post.published.limit(20)
# This creates N+1 queries:
# 1 query for posts
# N queries for each post's author
# N queries for each post's comments count
end
end

In the view:

<% @posts.each do |post| %>

<%= post.title %>

by <%= post.author.name %> # N+1 query here

<%= post.comments.count %> comments # N+1 query here

<% end %>

**After:**
```ruby
# app/controllers/posts_controller.rb
class PostsController < ApplicationController
  def index
    @posts = PostQuery.new.published_with_details.limit(20)
  end
end

# app/queries/post_query.rb
class PostQuery
  def initialize(relation = Post.all)
    @relation = relation
  end

  def published_with_details
    @relation.published
             .includes(:author, :tags)
             .left_joins(:comments)
             .select('posts.*, COUNT(comments.id) as comments_count')
             .group('posts.id')
             .order(created_at: :desc)
  end

  def by_author(author)
    @relation.where(author: author)
  end

  def recent(days = 7)
    @relation.where('created_at > ?', days.days.ago)
  end
end

# Or using a scope in the model:
# app/models/post.rb
class Post < ApplicationRecord
  belongs_to :author, class_name: 'User'
  has_many :comments
  has_and_belongs_to_many :tags

  scope :published, -> { where(published: true) }
  scope :with_details, -> { 
    includes(:author, :tags)
      .left_joins(:comments)
      .select('posts.*, COUNT(comments.id) as comments_count')
      .group('posts.id')
  }
end

5. Callback Hell → Service Objects and Observers

Before:
```ruby
class Order < ApplicationRecord
belongsto :user
hasmany :items

beforesave :calculatetotal
aftercreate :sendconfirmationemail
aftercreate :updateinventory
aftercreate :createshipment
aftercreate :updateuserstats
aftercreate :notifyadminiflargeorder
afterupdate :handlestatuschange

private

def calculatetotal
self.total = items.sum { |item| item.price * item.quantity }
self.tax = total * 0.08
self.totalwith_tax = total + tax
end

def sendconfirmationemail
OrderMailer.confirmation(self).deliver_now
end

def update_inventory
items.each do |item|
item.product.decrement!(:stock, item.quantity)
end
end

def create_shipment
Shipment.create!(order: self, status: 'preparing')
end

def updateuserstats
user.increment!(:totalorders)
user.update!(lastorder_at: Time.current)
end

def notifyadminiflargeorder
AdminMailer.largeorderalert(self).deliver_now if total > 1000
end

def handlestatuschange
if statuschanged? && status == 'shipped'
OrderMailer.shippednotification(self).delivernow
TrackingService.createtracking_number(self)
end
end
end
```

After:
```ruby

app/models/order.rb

class Order < ApplicationRecord
belongsto :user
hasmany :items

beforesave :calculatetotal

# Use a service object instead of callbacks
def process!
OrderProcessingService.new(self).process!
end

private

def calculate_total
calculator = OrderTotalCalculator.new(self)
calculator.calculate

self.total = calculator.total
self.tax = calculator.tax
self.totalwithtax = calculator.totalwithtax
end
end

app/services/orderprocessingservice.rb

class OrderProcessingService
def initialize(order)
@order = order
end

def process!
ActiveRecord::Base.transaction do
sendconfirmationemail
updateinventory
createshipment
updateuserstats
notifyadminif_needed
end
end

private

attr_reader :order

def sendconfirmationemail
OrderMailer.confirmation(order).deliver_later
end

def update_inventory
InventoryUpdateService.new(order).call
end

def create_shipment
ShipmentCreationService.new(order).call
end

def updateuserstats
UserStatsUpdateService.new(order.user, order).call
end

def notifyadminifneeded
AdminNotificationService.new(order).notifyiflargeorder
end
end

app/observers/order_observer.rb (if using observers)

class OrderObserver < ActiveRecord::Observer
def afterupdate(order)
return unless order.statuschanged?

case order.status
when 'shipped'
handleshipment(order)
when 'cancelled'
handlecancellation(order)
end
end

private

def handleshipment(order)
OrderMailer.shippednotification(order).deliverlater
TrackingService.createtracking_number(order)
end

def handlecancellation(order)
InventoryRestoreService.new(order).call
RefundService.new(order).processrefund
end
end
```

View Refactoring

6. Complex Views → Partials and Helpers

Before:
```erb
<!-- app/views/users/show.html.erb -->


<% if @user.avatar.present? %>
<%= imagetag @user.avatar, class: "avatar", size: "100x100" %>
<% else %>

<%= @user.firstname.first + @user.last_name.first %>

<% end %>

<%= @user.firstname %> <%= @user.lastname %>

<% if @user.active? %>
Active
<% else %>
Inactive
<% end %>

Member since:
<% if @user.createdat > 1.year.ago %>
<%= @user.createdat.strftime("%B %Y") %>
<% else %>
<%= @user.created_at.strftime("%B %d, %Y") %>
<% end %>

<%= @user.posts.count %>
Posts


<%= @user.comments.count %>
Comments



<% totallikes = @user.posts.joins(:likes).count %>
<%= totallikes %>

Likes Received

Recent Posts
<% @user.posts.recent.limit(5).each do |post| %>

<%= linkto post.title, post %>
<%= truncate(post.content, length: 150) %>

Posted <%= timeagoinwords(post.created_at) %> ago
• <%= post.comments.count %> comments
• <%= post.likes.count %> likes


<% end %>


```

After:
```erb
<!-- app/views/users/show.html.erb -->

<%= render 'userheader', user: @user %>
<%= render 'userstats', user: @user %>
<%= render 'recent_posts', posts: @user.posts.recent.limit(5) %>

<!-- app/views/users/userheader.html.erb -->

<%= useravatar(user, size: 100) %>
<%= user.displayname %>
<%= userstatusbadge(user) %>
Member since: <%= membership_duration(user) %>

<!-- app/views/users/userstats.html.erb -->

<%= render 'statitem', number: user.posts.count, label: 'Posts' %>
<%= render 'statitem', number: user.comments.count, label: 'Comments' %>
<%= render 'statitem', number: user.totallikes_received, label: 'Likes Received' %>

<!-- app/views/users/statitem.html.erb -->

<%= number %>
<%= label %>

<!-- app/views/users/recentposts.html.erb -->

Recent Posts
<%= render posts if posts.any? %>

<!-- app/views/posts/post.html.erb -->

<%= linkto post.title, post %>
<%= truncatetext(post.content, 150) %>
<%= render 'postmeta', post: post %>

<!-- app/views/posts/postmeta.html.erb -->

Posted <%= friendlytimestamp(post.createdat) %>
• <%= pluralize(post.comments.count, 'comment') %>
• <%= pluralize(post.likes.count, 'like') %>

```

With corresponding helpers:
```ruby

app/helpers/users_helper.rb

module UsersHelper
def useravatar(user, size: 50)
if user.avatar.present?
imagetag user.avatar, class: "avatar rounded-circle", size: "#{size}x#{size}"
else
content_tag :div, user.initials,
class: "avatar-placeholder rounded-circle",
style: "width: #{size}px; height: #{size}px;"
end
end

def userstatusbadge(user)
cssclass = user.active? ? "badge-success" : "badge-secondary"
statustext = user.active? ? "Active" : "Inactive"
contenttag :span, statustext, class: "badge #{css_class}"
end

def membershipduration(user)
if user.createdat > 1.year.ago
user.createdat.strftime("%B %Y")
else
user.createdat.strftime("%B %d, %Y")
end
end
end

Add to User model:

class User < ApplicationRecord
def displayname
"#{firstname} #{last_name}"
end

def initials
"#{firstname.first}#{lastname.first}".upcase
end

def totallikesreceived
posts.joins(:likes).count
end
end
```

Refactoring Best Practices

1. Start Small

• Refactor one method at a time
• Make small, incremental changes
• Test after each change

2. Identify Code Smells

• Long methods (> 10 lines)
• Large classes (> 100 lines)
• Duplicate code
• Long parameter lists
• Complex conditionals

3. Use Rails Conventions

• Follow the Rails Way
• Use existing Rails patterns
• Leverage Rails' built-in methods

4. Always Test

# Before refactoring, write tests to ensure behavior doesn't change
class OrderTest < ActiveSupport::TestCase
  test "order total calculation includes tax" do
    order = orders(:sample_order)
    order.calculate_total

    expected_total = order.items.sum { |item| item.price * item.quantity }
    expected_tax = expected_total * 0.08

    assert_equal expected_total, order.total
    assert_equal expected_tax, order.tax
    assert_equal expected_total + expected_tax, order.total_with_tax
  end
end

5. Refactor in Steps

1. Extract Method: Break large methods into smaller ones
2. Extract Class: Move related methods to a new class
3. Replace Conditional with Polymorphism: Use inheritance or strategy pattern
4. Move Method: Move methods to more appropriate classes
5. Rename: Use clear, descriptive names

Remember: refactoring is about improving code structure while maintaining functionality. Always have good test coverage before refactoring, and make small changes incrementally.