Stored Procedure calculating employee earnings

Question

I have following stored procedure in a C# win forms application which calculates employee earnings based on attendance as follows.

Note that a shift is 12 hours and employees mark attendance for in and out of each shifts. Also salary period is from beginning to end of a month (1^st to 28^st / 30^th / 31^st).

Related tables are:

• Employee (emp_id, initials, surname, basic_sal, budj_allowance)
• Attendance (emp_id, in_time, out_time, shift)
• Rank (rank_id, shift_rate)

Calculations

1. Work Days - This is the number of days a particular employee has worked and this value is taken from Attendance table.

2. Day Offs - An employee is entitled for maximum of 4 day offs for a month and if more than four days have been taken by an employee, remaining days will be marked as "Leave days".

3. No of Extra Shifts - This value is taken by this formula.

   [Total Shifts - total days worked]
   

4. Basic Salary - This is taken from employee master table

5. Budgetary Allowance - All employees are paid Rs.1,000/- as budgetary allowance

6. No Pay Days - This is calculated from the formula

   [(No of days in the month-04) - days worked]
   

7. Less No Pay Amount - This is calculated from the formula.

   [((Basic Salary + Budgetary Allowance) / (No of Days in the month-04)) x No Pay Days]

8. Amount for the EPF - This is calculated from the formula

   [Basic Salary + Budgetary Allowance - Less No Pay Amount]
   

9. Overtime Amount - This is calculated from the formula

   [Amount for the EPF - (Extra Shift Rate x Work Days)]
   

CREATE PROCEDURE [dbo].[sp_Earnings] @fromDate datetime, @toDate datetime 
    -- Add the parameters for the stored procedure here

AS
BEGIN
-- Declaring a variable to hold on of days in the month.
DECLARE @No_of_days int
SELECT @No_of_days = DATEDIFF(day,@fromDate,DATEADD(day,1,(@toDate)))

-- Declaring a constant to hold no of off days allowed in a month
DECLARE @Day_offs_allowed int
SELECT @Day_offs_allowed=4

--This is a reference to identify month and year of everyrecord. example - **"APR2014"**

DECLARE @SalRef char(20)
SELECT @SalRef= REPLACE(STUFF(CONVERT(varchar(12),CONVERT(date,@fromDate,107),106),1,3,''),' ','')

    -- SET NOCOUNT ON added to prevent extra result sets from
    -- interfering with SELECT statements.
    SET NOCOUNT ON;

    -- Insert statements for procedure here


SELECT     Employee.Emp_ID, Employee.Initials + ', ' + Employee.Surname AS Name, 
                        COUNT(DISTINCT CONVERT(DATE, Attendance.in_time)) AS work_days,

                        CASE WHEN (@No_of_days - (COUNT(DISTINCT CONVERT(DATE, Attendance.in_time))) >= @Day_offs_allowed) 
                        THEN @Day_offs_allowed 
                        ELSE (@No_of_days - (COUNT(DISTINCT CONVERT(DATE, Attendance.in_time)))) 
                        END AS day_offs, 

                        CASE WHEN (@No_of_days - (COUNT(DISTINCT CONVERT(DATE, Attendance.in_time))) >= @Day_offs_allowed) 
                        THEN @No_of_days - (COUNT(DISTINCT CONVERT(DATE, Attendance.in_time))) - @Day_offs_allowed 
                        ELSE 0 
                        END AS leave_days, 

                        COUNT(Attendance.shift) - COUNT(DISTINCT CONVERT(DATE, Attendance.in_time)) AS extra_shifts, 
                        Rank.Shift_Rate,  
                        (COUNT(Attendance.shift) - COUNT(DISTINCT CONVERT(DATE, Attendance.in_time)))* rank.Shift_Rate AS Extra_Shift_Amount, 
                        employee.Basic_Sal, 
                        employee.budj_allowance,
                        (@No_of_days-@Day_offs_allowed)- COUNT(DISTINCT CONVERT(DATE, Attendance.in_time)) AS no_pay_days,
                        CONVERT(DECIMAL(10,2),(((employee.basic_sal+employee.budj_allowance) / (@No_of_days-@Day_offs_allowed) )) * ((@No_of_days-@Day_offs_allowed)- COUNT(DISTINCT CONVERT(DATE, Attendance.in_time)))) AS less_no_pay_amt, 
                        employee.basic_sal+employee.budj_allowance-CONVERT(DECIMAL(10,2),((employee.basic_sal+employee.budj_allowance) / (@No_of_days-@Day_offs_allowed) ) * ((@No_of_days-@Day_offs_allowed)- COUNT(DISTINCT CONVERT(DATE, Attendance.in_time))))AS amt_for_epf, 
                        CONVERT(DECIMAL(10,2),((Rank.Shift_Rate*(COUNT(DISTINCT CONVERT(DATE, Attendance.in_time))))-((((employee.basic_sal+employee.budj_allowance)-(((employee.basic_sal+employee.budj_allowance) / (@No_of_days-@Day_offs_allowed)) * (@No_of_days-@Day_offs_allowed- COUNT(DISTINCT CONVERT(DATE, Attendance.in_time))))))))) AS over_time_amt, 
                        @salRef AS Reference 
FROM         Employee INNER JOIN
                      Attendance ON Employee.Emp_ID = Attendance.EID INNER JOIN
                      Point ON Attendance.PID = Point.PID INNER JOIN
                      Rank ON Employee.Rank = Rank.Rank_ID 
WHERE Attendance.in_time BETWEEN CONVERT(DATETIME, @fromDate, 102) AND CONVERT(DATETIME, @toDate, 102)                       
GROUP BY Employee.Emp_ID, Employee.Initials + ', ' + Employee.Surname, Rank.Shift_Rate, Employee.Basic_Sal, Employee.budj_allowance  
ORDER BY Employee.Emp_ID

END

Questions:

1. Can this be further optimized?
2. Are there any notable flows?
3. Is a stored procedure suitable for this requirement?

Answer 1

Here are my thoughts on your proc.

The Good

Good job on commenting out sections that are not so obvious to figure out. I had very little difficulty understanding it, your code is much cleaner than the average SQL post on this site. I would say a stored procedure is the correct type of database object for this, since it sounds like it will be called regularly based on its nature.

Improvements

1. Even though SQL engine is set up to where you don't always need to use delimiter ; it is good practice to explicitly use them. DECLARE @No_of_days int becomes DECLARE @No_of_days int; etc. throughout.

2. To avoid errors when creating a proc, it is a good idea to DROP PROCEDURE IF EXISTS [dbo].[sp_Earnings]; for instance.

3. This may just be personal preference, but I think SET is less ambiguous to set variables, so SELECT @Day_offs_allowed=4; would become SET @Day_offs_allowed=4;. This helps to differentiate them from nested queries and such.

4. I think this is a bit odd:
   SELECT @SalRef= REPLACE(STUFF(CONVERT(varchar(12),CONVERT(date,@fromDate,107),106),1,3,''),' ','').
   I would be tempted to instead use
   SET @SalRef = CONCAT( DATEPART(Mm, @fromDate), '/', DATEPART(Yy, @fromDate) ).
   This reference will look slightly different, e.g. "04/2014" but achieve the same purpose more elegantly plus you can sort them in order more easily numerically.

5. A point on formatting. When using long nested statements it is good practice to use line break and tabs to make it easier to read. For example:

   (COUNT(Attendance.shift) - COUNT(DISTINCT CONVERT(DATE, Attendance.in_time)))* rank.Shift_Rate AS Extra_Shift_Amount
   

   Becomes:

   (COUNT
       (Attendance.shift) - COUNT(        
           DISTINCT CONVERT(DATE, Attendance.in_time)    
       )        
   )* rank.Shift_Rate AS Extra_Shift_Amount,
   

Other than that I think your code is good.