Selecting the number of working days minus weekend days and UK bank holidays

Question

I am fairly new to coding TSQL script and am looking for some second view on my script. The goal here is to to pull in some data and amongst that data to show a field with a counting the number of working days since the order date only (i.e. minus weekend days and UK bank holidays). The SELECT script below is working as expected in that it shows results and is therefore calling the fn_WorkDays function. What would be helpful is to have a second glance of the function itself as a double check/review. The last part of the function is looking at a date field of a table that contains all of the bank holidays.

I think the script is working okay as I have checked a few going back to January, so I'm looking to get the double check to highlight any unforeseen potential errors or even code improvements.

SELECT 
cir.[PW Number] AS PWNum
--Get the correct number of working dayts since the order date by using the fn_WorkDays function
,CONVERT(VARCHAR(10),singleended2.dbo.fn_WorkDays(cir.[Install Date], GETDATE()),103) AS NumberOfDaysSinceOrderDate 
,CONVERT(VARCHAR(10), ISNULL(cir.[Install Date],'01/01/1900'),103) AS  OrderDate--Get the order dates
,ISNULL(cirRep.CurrentStage, 'Not Set') AS CurrentStage
,cir.[ID] as CircuitID
FROM Quotebase.dbo.Circuits cir
    LEFT JOIN Quotebase.dbo.CircuitReports cirRep ON Cir.[PW Number] = CirRep.PWNumber
WHERE Cir.Status='New Circuit Order' 
ORDER BY Cir.[PW Number]

Here is the function script that is called:

ALTER FUNCTION dbo.fn_WorkDays (@StartDate AS DATETIME, @EndDate AS DATETIME)
--Define the output data type.
RETURNS INT
AS
--Calculate the RETURN of the function.
BEGIN
    RETURN (
     SELECT
        (DATEDIFF(dd,@StartDate, @EndDate)+1)--Start with total number of days including weekends +1 Includes the day run
        -(DATEDIFF(wk,@StartDate, @EndDate)*2)--Subtact 2 days for each full weekend
        -(CASE WHEN DATENAME(dw, @StartDate) = 'Sunday' --If StartDate is a Sunday, Subtract 1
            THEN 1 
            ELSE 0 
        END) 
        -(CASE WHEN DATENAME(dw, @EndDate) = 'Saturday'--If EndDate is a Saturday, Subtract 1 
            THEN 1 
            ELSE 0 
        END)
        --Now check if there are any bank holidays between the start and end and remove that amount 
        - (SELECT COUNT(ivm.[Date])
          FROM [EUROPEVUK386].[InvoiceManagement].[dbo].[tblBankHolidays] ivm
          WHERE ivm.[Date] BETWEEN @StartDate AND @EndDate)     
          --WHERE ivm.[Date] BETWEEN '2006-04-14' AND '2006-05-29')     
)
 END
GO

Answer 1

1. ISNULL, which you use a few times, is valid T-SQL, but there is an argument for using the more portable COALESCE instead. They're not straight aliases, though, so consider whether this is appropriate.

2. Comparing the two lines

   ,CONVERT(VARCHAR(10),singleended2.dbo.fn_WorkDays(cir.[Install Date], GETDATE()),103) AS NumberOfDaysSinceOrderDate 
   ,CONVERT(VARCHAR(10), ISNULL(cir.[Install Date],'01/01/1900'),103) AS  OrderDate--Get the order dates
   

   you assume in the second line that cir.[Install Date] might be null, but the first line passes it directly to a function which has no special-casing for that situation. Is this intentional?

3. I think you're overdoing things in

   CONVERT(VARCHAR(10), ISNULL(cir.[Install Date],'01/01/1900'),103)
   

   You're trying to convert the date to a dd/mm/yyyy string, but if it's NULL you take a string in that format, convert it with whatever the default date format happens to be (although since the day and month are the same you have good odds of getting the right result), and then convert it back. Why not push in the CONVERT and handle the fallback outside?

4. You're making assumptions about locale-related settings inside the function. It's understandable that you hard-code an assumption about which days are the weekends: you're only interested in one culture, so that's a business-level setting. But the localisation of the values returned by DATENAME is a per-connection setting. You might think that your function will always be executed in an English locale, and you might even be right, but it's a small change to replace DATENAME with DATEPART, and it makes the code slightly more robust.

   (I've had experience of maintaining a system built for a British organisation but which relied implicitly on using a US locale; since my dev machine has a UK locale, lots of things broke. Don't discount the possibility that in future your code will be maintained by someone from another country who configures their dev machine differently - or even that the production machine might be moved to a server with a different locale).

5. For the benefit of future maintenance programmers, who may not be SQL-Server experts or domain experts on UK bank holidays, it would be nice to add some comments pointing out that DATEDIFF ignores @@DATEFIRST in order to be deterministic, and that a bank holiday will never be on a weekend.

Answer 2

I did notice in your Function you subtract a day from Saturday if it is an End Date, but not 2 days from the end date if it is Sunday, and not 2 days from the start date if it is Sunday or 1 day from the start date if it is Saturday.

I assume that you want to do these things to make sure you aren't trying to include the weekends especially since you are using Between and that is inclusive, so it would include the start date and the end date