User Defined FUNCTION
-
Hey am new to UDF's and am looking over someone else code and i need to know whats going on with in the code
/* Am not sure what going on with the 2 variables @StartDate datetime, @EndDate datetime as the call to the function is this
dbo.udf_business_days(dbo.tbl_metrics_grca_fl.open_dt, dbo.svw_fl_unique_g_serv_sn.open_dt)
CREATE FUNCTION dbo.udf_bus_days(@StartDate datetime, @EndDate datetime)
RETURNS int
AS
BEGIN
DECLARE @days int, @we_days int, @Diff int
Set @days = datediff(dd, @StartDate, @EndDate)
Set @we_days = datediff(wk, @StartDate, @EndDate)*2
If dbo.udf_date_only(@StartDate) = dbo.udf_date_only(@EndDate)
Begin
If dbo.udf_time_only(@StartDate) < dbo.udf_time_only(@EndDate)
Begin
Set @Diff = 1
End
Else
Set @Diff = 0
End
If dbo.udf_date_only(@StartDate) < dbo.udf_date_only(@EndDate)
Begin
If dbo.udf_time_only(@StartDate) < dbo.udf_time_only(@EndDate)
Begin
Set @Diff = (datediff(dd, @StartDate, @EndDate)) - @we_days + 1
End
If dbo.udf_time_only(@StartDate) >= dbo.udf_time_only(@EndDate)
Begin
Set @Diff = (datediff(dd, @StartDate, @EndDate)) - @we_days
End
End
If @Diff < 0
Begin
Set @Diff = 0
End
RETURN @Diff
END
-
Hello,
the function has no comments? That "someone else" should learn to put comments in permanent code always - even if it seems to be easy to understand.
OK, I can see more or less what happens, though I don't know what the tables dbo.tbl_metrics_grca_fl, dbo.svw_fl_unique_g_serv_sn are and what the dates mean.
@days is declared and set, but not used later; it is the number of days between the two dates
@we_days will probably mean "weekend days", because they are two in each week (number of weeks * 2). Used later to find out, how many working days elapsed from start to end date. Ignores national holidays and generally I'm not sure it gives correct result... seems to be more of estimate than precise number.
Now, the code compares always first the date part of the dates and then separately the time part (I don't have the code of called functions, so I can only guess that from their names - one will strip away the time part and the other the date part from a datetime format), and depending on situation returns some integer value that is probably meant to be a number of working days between start and end date.
If both dates are from the same day, function returns 1 for those where Start < End, for those with Start>=End it returns 0.
If Start is from a day that is older than End, function returns (more or less) the number of working days between the two dates. If the Start time is lower than End time, e.g. start = 2005.01.01. 10:00:00 and end = 2005.08.12 11:00:00, one day is added.
Again, I'd like to emphasize that I'm only guessing what the author wanted to do, and that I didn't have time to check the function properly, to see what it really does under various circumstances.
If you need some more help, please go on and ask!
/EDIT : I tested the function, and - given that the code in called functions is OK - it seems to calculate the number of business days correctly.. with the assumption that national holidays are considered business days, if they fall on workday/
-
Thanks for your help, regards the comments i've added in comments into the code now.. i've 65 UDF in the SQL DB and none of them have comments in them.... am only new to SQL UDF but i always added comments into code, i will possible need your help in the futire.
Many thanks.
-
I really feel with you, to have entirely undocumented code in a DB is a pain in the... you know where. Well, for the start you can comment 3 UDFs (this one and those that extract parts from a datetime value (dbo.udf_date_only, dbo.udf_time_only), and I'm sure it will go fine once you see more of the code and begin to understand not only SQL functions, but also the way the programmer was thinking. If you're stuck, call for help - we will try to decipher the more complicated parts of code.
good luck,
Vladan
-
There is no return for dbo.udf_date_only(@StartDate) > dbo.udf_date_only(@EndDate)
And you repeat the same checks again and again. Not really good for performance.
This thing does the same stuff but it's shorter and it does not miss cases:
ALTER FUNCTION dbo.udf_bus_days(@StartDate datetime, @EndDate datetime)
RETURNS int
AS
BEGIN
DECLARE @days int, @we_days int, @Diff int
Set @days = datediff(dd, @StartDate, @EndDate)
IF @Days < 0
Return 0
Set @we_days = datediff(wk, @StartDate, @EndDate)*2
If dbo.udf_time_only(@StartDate) < dbo.udf_time_only(@EndDate)
Set @Diff = 1
Else
Set @Diff = 0
Set @Diff = @days - @we_days + @Diff
If @Diff < 0
Set @Diff = 0
RETURN @Diff
END
GO
_____________
Code for TallyGenerator -
Francis, I know exactly what you're going through... I just inherited yet another monster set of databases and I think among all the SQL procs, views, and udfs, there might be a dozen comments...
I also see they blessed you with a UDF that calls another UDF... a real "Bozo-No-No" in my book...
I felt so bad for you I thought I'd give you a piece of the code I put into production to get you started... it's a "calulate work days" function like the one you posted with a couple of additional benys and full documentation. If you decide to use it, you'll need to change the name of the function to match your's so no code is broken... feel free to modify as you see fit.
CREATE FUNCTION dbo.fn_WorkDays
/***************************************************************************************
Purpose:
1. Given any valid start date and end date, this function will calculate and return
the number of workdays (Mon - Fri).
2. Given only a valid start date (end date has DEFAULT in it), this function will
return a 1 if the start date is a weekday and a 0 if not a weekday.
Usage:
1. dbo.fn_WorkDays(@StartDate,@EndDate)
2. dbo.fn_WorkDays(@StartDate,DEFAULT) --Always returns 1 or 0
3. dbo.fn_WorkDays(@EndDate,@StartDate)
4. dbo.fn_WorkDays(@StartDate,@StartDate) --Always returns 1 or 0
5. dbo.fn_WorkDays(@EndDate,@EndDate) --Always returns 1 or 0
Notes:
1. Holidays are NOT considered.
2. Because of the way SQL Server calculates weeks and named days of the week, no
special consideration for the value of DATEFIRST is given. In other words, it
doesn't matter what DATEFIRST is set to for this function.
3. If the input dates are in the incorrect order, they will be reversed prior to any
calculations.
4. Only whole days are considered. Times are NOT used.
5. The number of workdays INCLUDES both dates
6. Inputs may be literal representations of dates, datetime datatypes, numbers that
represent the number of days since 1/1/1900 00:00:00.000, or anything else that can
be implicitly converted to or already is a datetime datatype.
7. Undocumented: The DATEPART(dw,date) does not actually count weeks... It counts the
transition to a Sunday regardless of the DATEFIRST setting. In essence, it counts
only whole weekends in any given date range.
8. This UDF does NOT create a tally table or sequence table to operate. Not only is
it set based, it is truly "tableless".
Error Indications:
1. If either the @StartDate or the @EndDate parameter is an invalid date, the
following error is returned...
"Server: Msg 242, Level 16, State 3, Line 3
The conversion of a char data type to a datetime data type resulted in an
out-of-range datetime value."
2. If either the @StartDate or the @EndDate parameter is a string not resembling a
date, the following error is returned...
"Server: Msg 241, Level 16, State 1, Line 3
Syntax error converting datetime from character string."
3. If only one parameter is passed, the following error is returned...
"Server: Msg 313, Level 16, State 2, Line 3
An insufficient number of arguments were supplied for the procedure or
function dbo.fn_WorkDays."
Revisions:
Rev 00 - 12/12/2004 - Jeff Moden - Initial creation and test.
Rev 01 - 12/12/2004 - Jeff Moden - Load test, cleanup, document, release.
Rev 02 - 12/26/2004 - Jeff Moden - Return NULL if @StartDate is NULL or DEFAULT and
modify to be insensitive to DATEFIRST settings.
***************************************************************************************/
--======================================================================================
-- Presets
--======================================================================================
--===== Define the input parameters (ok if reversed by mistake)
(
@StartDate DATETIME,
@EndDate DATETIME = NULL --@EndDate replaced by @StartDate when DEFAULTed
)
--===== Define the output data type
RETURNS INT
AS
--======================================================================================
-- Calculate the RETURN of the function
--======================================================================================
BEGIN
--===== Declare local variables
--Temporarily holds @EndDate during date reversal
DECLARE @Swap DATETIME
--===== If the Start Date is null, return a NULL and exit
IF @StartDate IS NULL
RETURN NULL
--===== If the End Date is null, populate with Start Date value
-- so will have two dates (required by DATEDIFF below)
IF @EndDate IS NULL
SELECT @EndDate = @StartDate
--===== Strip the time element from both dates (just to be safe) by converting
-- to whole days and back to a date. Usually faster than CONVERT.
-- 0 is a date (01/01/1900 00:00:00.000)
SELECT @StartDate = DATEADD(dd,DATEDIFF(dd,0,@StartDate),0),
@EndDate = DATEADD(dd,DATEDIFF(dd,0,@EndDate) ,0)
--===== If the inputs are in the wrong order, reverse them
IF @StartDate > @EndDate
SELECT @Swap = @EndDate,
@EndDate = @StartDate,
@StartDate = @Swap
--===== Calculate and return the number of workdays using the
-- input parameters. This is the meat of the function.
-- This is really just one formula with a couple of parts
-- that are listed on separate lines for documentation
-- purposes.
RETURN (
SELECT
--Start with total number of days including weekends
(DATEDIFF(dd,@StartDate,@EndDate)+1)
--Subtact 2 days for each full weekend
-(DATEDIFF(wk,@StartDate,@EndDate)*2)
--If StartDate is a Sunday, Subtract 1
-(CASE WHEN DATENAME(dw,@StartDate) = 'Sunday'
THEN 1
ELSE 0
END)
--If EndDate is a Saturday, Subtract 1
-(CASE WHEN DATENAME(dw,@EndDate) = 'Saturday'
THEN 1
ELSE 0
END)
)
END
GO
--Jeff Moden
RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.Change is inevitable... Change for the better is not.
Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally) -
Hey Jeff, thanks for the code, am new to UDF so i think its best i stick with what i have for the moment, I've added in comments into the code, here whats it looks like now, i hope i just did not over do the comments ..
Francis
/*
This function accepts 2 dates a start date and a end date, it compares the dates and time
by using the 2 functions dbo.udf_date_only & dbo.udf_time_only
It returns the number of days excluding weekend days
*/
CREATE FUNCTION dbo.udf_bus_days(@StartDate datetime, @EndDate datetime)
RETURNS int
AS
BEGIN
DECLARE @days int, @we_days int, @Diff int
-- Gets the number of days between start date and a end date dd = days
Set @days = datediff(dd, @StartDate, @EndDate)
-- Gets the number of weeks between start date and a end date wk = weeks Multipe by 2
Set @we_days = datediff(wk, @StartDate, @EndDate)*2
-- If the start date and a end date are equal
If dbo.udf_date_only(@StartDate) = dbo.udf_date_only(@EndDate)
Begin
-- If time on the start date is less than the end date
If dbo.udf_time_only(@StartDate) < dbo.udf_time_only(@EndDate)
Begin
Set @Diff = 1
End
Else
Set @Diff = 0
End
-- If the start date is less than the end date
If dbo.udf_date_only(@StartDate) < dbo.udf_date_only(@EndDate)
Begin
-- If time on the start date is less than the end date
If dbo.udf_time_only(@StartDate) < dbo.udf_time_only(@EndDate)
Begin
-- Let the return value @Diff = the number of days "dd" between start date and a end date take away the @we_days + 1
Set @Diff = (datediff(dd, @StartDate, @EndDate)) - @we_days + 1
End
-- If time on the start date is greater or = than the end date
If dbo.udf_time_only(@StartDate) >= dbo.udf_time_only(@EndDate)
Begin
-- Let the return value @Diff = the number of days "dd" between start date and a end date take away the @we_days
Set @Diff = (datediff(dd, @StartDate, @EndDate)) - @we_days
End
End
-- If for some reason the value is less than 0 just set the @Diff = 0
If @Diff < 0
Begin
Set @Diff = 0
End
RETURN @Diff
END
-
Hi Francis,
I can understand that you don't want to change anything right now, when you're only learning how everything works. I'd do the same... it is better to map everything first and only then start improving parts that need it. This is also the reason why I didn't bother to rewrite the function - I concentrated on explaining how the posted code works.
This version looks much better than the previous without comments, however, I can't help myself observing, that the comments are not the best. Of course, everyone has their own way to write comments, which they claim to be the best or even the only correct way
. IMHO the comments should not repeat what is self-evident for anyone knowing the language - they should explain WHY it is done, and add some info that can not be seen at first glance. I hope you don't mind if I point out what I mean...
-- Gets the number of weeks between start date and a end date wk = weeks Multipe by 2
This is simply the code translated into words. Why is the number of weeks multiplied by 2? I would write something like
/*@we_days=number of weekend days between dates
wk = weeks; 2 weekend days per week */
Set @we_days = datediff(wk, @StartDate, @EndDate)*2
Another example...
-- Let the return value @Diff = the number of days "dd" between start date and a end date take away the @we_days + 1
Wording is not precise : do you take away (@we_days+1), or do you take away @we_days and then add 1? Why do you add 1? Again, this is how I'd write comments:
/*New day considered started every precise 24 hours from StartDate time; compare time-parts of dates to see if this border was crossed. If yes, add one day*/
If dbo.udf_time_only(@StartDate) < dbo.udf_time_only(@EndDate)
Begin
Set @Diff = (datediff(dd, @StartDate, @EndDate)) - @we_days + 1
End
Well, as I already said, opinions about how to write comments may differ. I just thought it could help you if I explain mine a bit... It is up to you to decide whether it makes sense for you or not.
HTH, Vladan
Viewing 8 posts - 1 through 7 (of 7 total)
You must be logged in to reply to this topic. Login to reply