Jack: Employee Salary Calculator exercise

employee.rb

class Employee
	attr_reader :name, :month_salary
	def initialize(args)
		@name = args[:name]
		@month_salary = args[:month_salary]
	end

	def vat
		0.20
	end

	def gross_salary_pa
		month_salary * 12
	end

	def net_salary_pa
		gross_salary_pa - gross_salary_pa * vat
	end
end

employee_test.rb

require "./app/employee.rb"
require "test/unit"

class EmployeeTest < Test::Unit::TestCase
	def test_instantiation
		employee = Employee.new(name: "Jack", month_salary: 1000)
		assert_equal(Employee, employee.class)
		assert_equal("Jack", employee.name)
		assert_equal(1000, employee.month_salary)
		assert_equal(12000, employee.gross_salary_pa)
		assert_equal(9600, employee.net_salary_pa)
	end
end

payroll_employee.rb

require "./app/employee.rb"

class PayrollEmployee < Employee

	def gross_salary_pa
		month_salary * 13
	end
end

payroll_employee_test.rb

require "./app/payroll_employee.rb"
require "test/unit"

class PayrollEmployeeTest < Test::Unit::TestCase

	def payroll_emp
		payroll_emp = PayrollEmployee.new(name: "Jack", month_salary: 1000)
	end
	def test_intantiation
		assert_equal("Jack", payroll_emp.name)
		assert_equal(1000, payroll_emp.month_salary)
	end

	def test_salary
		assert_equal(13000, payroll_emp.gross_salary_pa)
		assert_equal(10400, payroll_emp.net_salary_pa)
	end
end

sales_employee_test.rb

require "./app/sales_employee.rb"
require "test/unit"

class SalesEmployeeTest < Test::Unit::TestCase
	def test_intantiation
		sales_emp = SalesEmployee.new(name: "Jack", month_salary: 1000, sales: 10000)
		assert_equal(SalesEmployee, sales_emp.class)
		assert_equal("Jack", sales_emp.name)
		assert_equal(1000, sales_emp.month_salary)
		assert_equal(10000, sales_emp.sales)
		assert_equal(2000, sales_emp.bonus)
		assert_equal(14000, sales_emp.gross_salary_pa)
		assert_equal(11200, sales_emp.net_salary_pa)
	end
end

system.rb

require "./app/payroll_employee.rb"
require "./app/sales_employee.rb"

class System
	attr_reader :employees
	def initialize(args)
		@employees = args[:employees]
	end

	def emp_names
		list = []
		employees.each do |employee|
			names(list,employee)
		end
		list
	end

	def tot_gross_salary_pa
		total = 0
		employees.each do |employee|
			total += emp_gross_salary_pa(employee)
		end
		total
	end


	private

	def emp_name(employee)
		employee.name
	end
	def names(list,employee)
		list << emp_name(employee)
	end

	def emp_gross_salary_pa(employee)
		employee.gross_salary_pa
	end
end

system_test.rb

require "./app/system.rb"
require "test/unit"

class SystemTest < Test::Unit::TestCase

	def system
		System.new(employees: [
			PayrollEmployee.new(name: "Jack", month_salary: 1000),
			SalesEmployee.new(name: "Brad", month_salary: 1000, sales: 10000)
		])
	end

	def test_instantiation
		assert_equal(System, system.class)
	end

	def test_emp_names
		assert_equal(["Jack", "Brad"], system.emp_names)
	end

	def test_tot_salary
		assert_equal(27000, system.tot_gross_salary_pa)
	end
end

Public vs private

Without knowing more details about how you are planning on using this code, deciding what should be private and public is obviously very much up to you :-)
It feels to me however that a couple more methods could be private, eg the vat method on Employee. A couple of the instance variables that you make accessible via attr_reader could also possibly be private.

Use of constants

For constant values like VAT, I would consider not using a method but rather using a constant and potentially freezing it? Again, very much up to how you are expecting to handle that value in your objects :-)

Test structure

There are two things that I believe you can improve on the structure and meaning of your tests.

1. Structure
   A commonly used structure for organising the content of your tests is Arrange/Act/Assert. Most testing frameworks force you to organise your tests in that structure, and you are mostly doing so. The part that would make your tests more readable and descriptive is:

• if your tests were divided up more granularly
• if they were described in a more meaningful way

Enough waffle, some examples :-)

In the case of your SalesEmployee test you are actually testing a lot more than instantiation

def test_intantiation
        sales_emp = SalesEmployee.new(name: "Jack", month_salary: 1000, sales: 10000)
        assert_equal(SalesEmployee, sales_emp.class)
        assert_equal("Jack", sales_emp.name)
        assert_equal(1000, sales_emp.month_salary)
        assert_equal(10000, sales_emp.sales)
        assert_equal(2000, sales_emp.bonus)
        assert_equal(14000, sales_emp.gross_salary_pa)
        assert_equal(11200, sales_emp.net_salary_pa)
    end

This is actually testing seven different methods :-) and would be more readable if separated out in two or three tests with descriptions :-)
eg the first 5 lines of your test are looking at instantiation and checking your instance variables have been set properly during initialization.
the other lines seem to be testing other things and therefore can be separated out for extra readability.

Some possible refactorings in system.rb

1

employees.each do |employee|
  names(list,employee)
end

def names(list,employee)
  list << emp_name(employee)
end

is actually a reimplementation of a map.

employees.map do |employee|
  emp_name(employee)
end

Actually since emp_name is actually doing employee.name, I'm not quite sure why you would want that emp_name method there in the first place and therefore you could probably do:

employees.map do |employee|
  employee.name
end

which actually can be "golfed" down to:

employees.map(&:name)

2

The same kind of refactoring could be applied to the tot_gross_salary_pa method :-)

employees.map(&: gross_salary_pa)..reduce(:+)

Signing off now :-)
Thank you for sharing this with me!

My overall comment for this is well done!
I particularly liked that you took care of:

• writing carefully named methods
• thoroughly testing things
• thinking about your Employee vs SalesEmployee structure of inheritance

As all code things, there are always aspects that can be changed and improved but this is a very solid solution to the exercise 👍

Wow! Thanks Camille, that's more than expected!
I will study all you said properly and get back to you, you're the best! 😄

Many thanks again.
Jack