Daily Refactor #78: Replace Data Value in InvoiceController with Object

The Refactoring

Today’s refactoring cleans up some of the stubs left from last Friday.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20

class InvoiceController < ApplicationController
  def autofill
    @autofill = Autofill.new_from_params(params[:autofill])
 
    # TODO: should just access @autofill directly
    @p = @autofill.p
    @customer = @autofill.customer
    @date_from = @autofill.date_from
    @date_to = @autofill.date_to
    @activities = @autofill.activities
    @issues = @autofill.issues
    @total_time = @autofill.total_time
    @time_entries = @autofill.time_entries
    @total = @autofill.total
 
    respond_to do |format|
      format.js
    end
  end
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29

# app/views/invoices/autofill.js.rjs
page['invoice_amount'].value = @total
page['invoice_project_id'].value = @p.id
 
# Invoice description
description = ''
description << "h3. " + h(@p.name) + "\n\n"
description << h(@p.description) + "\n\n"
description << "h3. Work Completed\n\n"
 
@issues.each do |issue|
  description << "* #{issue.subject} (#{issue.status} ##{issue.id})\n"
end
 
# Total time
description << "\n"
description << "Total billable time: " + pluralize(@total_time, "hour")
 
description << " (" + h(@date_from) + " to " + h(@date_to) + ")"
 
page['invoice_description'].value = description
 
# Customer
if @customer.blank?
  page.alert l(:label_no_customer_on_project)
else
  page['invoice_customer_id'].value = @customer.id
  page.replace_html 'customer_name', h(@customer.name)
end

After

1
2
3
4
5
6
7
8
9

class InvoiceController < ApplicationController
  def autofill
    @autofill = Autofill.new_from_params(params[:autofill])
 
    respond_to do |format|
      format.js
    end
  end
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29

# app/views/invoices/autofill.js.rjs
page['invoice_amount'].value = @autofill.total
page['invoice_project_id'].value = @autofill.p.id
 
# Invoice description
description = ''
description << "h3. " + h(@autofill.p.name) + "\n\n"
description << h(@autofill.p.description) + "\n\n"
description << "h3. Work Completed\n\n"
 
@autofill.issues.each do |issue|
  description << "* #{issue.subject} (#{issue.status} ##{issue.id})\n"
end
 
# Total time
description << "\n"
description << "Total billable time: " + pluralize(@autofill.total_time, "hour")
 
description << " (" + h(@autofill.date_from) + " to " + h(@autofill.date_to) + ")"
 
page['invoice_description'].value = description
 
# Customer
if @autofill.customer.blank?
  page.alert l(:label_no_customer_on_project)
else
  page['invoice_customer_id'].value = @autofill.customer.id
  page.replace_html 'customer_name', h(@autofill.customer.name)
end

Review

By changing the data access so it goes through the @autofill instance, I don’t have to track as many instance variables in the controller and have the flexibility to change any of the variables’ implementation later. This will let me refactor that RJS away into a cleaner interface later.

Reference commit