Friday, February 26, 2010

Uncle Bob's Payroll Case Study: Use Case 2

---------------------------------------------------------
USE CASE 2: DELETING AN EMPLOYEE

Employees are deleted when a DelEmp transaction is received. The form of this transaction is as follows:

DelEmp [EmpID]

When this transaction is received, the appropriate employee record is deleted.

Alternative: Invalid or unknown EmpID
If the [EmpID] field is not structured correctly, or if it does not refer to a valid employee record, then the transaction is printed with an error message, and no other action is taken.
---------------------------------------------------------

A couple of new tests helped to drive my production code. Both were added to the RecordsManagerTest class, so I'll just post my additions:
class RecordsManagerTest < Test::Unit::TestCase
NON_EXISTENT_EMP_ID = 99

def test_delete_employee_deletes_employee_record
@employee = create_one_employee

assert_employee_was_deleted do
@records_manager.delete_employee(@employee.emp_id)
end
end

def test_delete_employee_emp_id_not_found
assert_equal RecordsManager::EMPLOYEE_NOT_FOUND_MESSAGE, @records_manager.delete_employee(NON_EXISTENT_EMP_ID)
end

private
def create_one_employee
@records_manager.add_employee(@new_employee_fields)
assert_employee_was_created
Employee.first
end

def assert_employee_was_created
assert_equal 1, Employee.count
end

def assert_employee_was_deleted
new_employee_count = Employee.count - 1
yield
assert_equal new_employee_count, Employee.count
end

end
As the tests indicate, a new method called 'delete_employee' is added to the RecordsManager class. Let's take a look at how that class looks now:

require 'app/models/salary'
require 'app/models/employee'

class RecordsManager
EMPLOYEE_NOT_FOUND_MESSAGE = "Employee not found. Cancelling delete."
def add_employee(fields)
new_employee = Employee.new(fields)
return if new_employee.save
error_message_for new_employee
end

def delete_employee(emp_id)
return if Employee.delete_by_emp_id(emp_id)
EMPLOYEE_NOT_FOUND_MESSAGE
end

private

def error_message_for(employee)
employee.errors.full_messages.to_s
end
end
You'll notice the addition of an error message constant, EMPLOYEE_NOT_FOUND_MESSAGE. Originally, I created a module for RecordsManager errors and added my one static error message. Then I realized how much overhead I added just to manage one message and reverted. YAGNI, for now.

In the 'delete_employee' function, the RecordsManager delegates to Employee by way of 'delete_by_emp_id.' Here's the new Employee class:
require 'rubygems'
require 'dm-core'
require 'dm-aggregates'
require 'dm-validations'
require 'app/models/salary'

DataMapper.setup(:default, :adapter => 'mysql',
:username => 'root',
:password => 'even-fish',
:database => 'payroll')
class Employee
include DataMapper::Resource

property :id, Serial
property :emp_id, Integer
property :name, String
property :address, Text

has 1, :salary

validates_present :emp_id, :name, :address
validates_is_unique :emp_id

def self.delete_by_emp_id(id)
if emp = Employee.first(:emp_id => id)
emp.destroy!
else return false
end
end
end
Thoughts:
  • I had 'assert_difference' in place in my first test function before I realized that this is a method defined by the ActiveSupport library. So I kept the spirit of assert_difference, knowing exactly the 'difference' I wanted to test, and created 'assert_employee_was_deleted.' I like this method because it yields to a block just like assert_difference does, allowing me to be expressive in my test while hiding unnecessary implementation details in the private method.
  • An interesting difference between ActiveRecord and DataMapper is in the comparison between the ActiveRecord 'find' functions and the DataMapper 'get' or 'first' functions. At first it felt funny to call Employee.first in order to find the employee record to delete. WAIT THIS STILL SEEMS FUNNY... Can't I use get with a key and value? Either way, I thought it important to add the uniqueness validation to :emp_id in order to feel safe in deleting an individual record.