I... Err... Wait... What?!

[FunkyDexter]

As a coder who often inherits other people work, I am not unfamiliar with the experience of coming across some strange piece of implementation that boggles my, admittedly already somewhat addled, mind. When I come across such a nugget I try to be generous. I try to be forgiving. Maybe the author had some deep insight which I lack; maybe they'd tried and exhausted all the more obvious solutions; maybe they were some abstract genius who could see the woman in the red dress and knew kung-fu... or maybe it was a Friday afternoon and they were drunk. We've all written some ingenious code on a Friday when we were drunk - I can forgive that.

But once in a very rare while I come across a glittering star of insanity, twinkling in the firmament of space. It's hard to offer the benefit of the doubt on these occasions because there can be no doubt. The sheer mind blasting craziness of the snippet I encounter precludes it. There can be no valid reaction to it beyond a dropped jaw and, possibly, a terrified sob.

On these occasions it would be remiss of me not to share. Not because I want to make you better coders but because I want to make you better human beings. It is only through the sheer horror engendered by the dawning knowledge that someone once wrote this and that they are still out there somewhere that we can truly learn how close we stand to the edge of the pit and what tentacled, bat winged monstrosities dwell therein, ready to be vomited forth upon the earth.

No... really... can anyone explain this to me. It's important because I'd like to sleep tonight:-

CREATE FUNCTION [dbo].[f_GetDuration] (@SystemId tinyint, @IsAnnual bit, @StoredDuration int, @StartDate datetime, @EndDate datetime) 
RETURNS int
AS 
BEGIN
 
    DECLARE @ActualDuration as int
 
    IF @IsAnnual = 1
    BEGIN
        SET @ActualDuration = 365
    END
    ELSE
    BEGIN
        SET @ActualDuration = @StoredDuration
    END
 
    RETURN @ActualDuration
 
END

I recommend giving it multiple read-throughs. It's only short but it's subtle and layered. I've read it several times and each time I notice another nuance. Some subtlety that I'd missed on previous iteration - presumably because I was blinded by the retina melting glare of the larger insanities. Like a work of classic fiction, it gives, and gives again.

[techgnome]

Could it be a product of refactoring over time? I've seen that happen... something gets refactored so many times, it gets lost in translation.

-tg

[FunkyDexter]

Possibly but I doubt it. There's comments at the top to support Jira change control. No actual comments were ever put in but you can see that there were only ever 3 changes made including the original implementation, all by the same person. Putting that aside, what sort of refactoring would ever lead to a function that does nothing but return either it's own input parameter or a hard value?

If you want to get to the real genius of this function, consider for a minute how it would be consumed... and whether, therefore, it should even exist. In particular, consider the performance impacts scalar functions are know to have.

[NeedSomeAnswers]

at least its nice and short , you maybe able to fault everything else but you cant fault its terseness

i could post code that would make you weep with its verboseness

[Shaggy Hiker]

I can see how this happened. Just look at the arguments to the function. That shows enthusiasm, optimism, and most of all, ambition. They started with a glorious vision: I shalt create a function that gives you the duration no matter what!!

The Begin and End show the very heights of their ambitious undertaking, though they are only the sad remnants of a once glorious attempt. Unfortunately, the user encountered the mess that is dates. The more they struggled, the tighter the bounds became, until, exhausted, they gave up. Their final comment, before moving on to something manageable, was only two words, the first of which started with F, and the latter of which was "it". Those words, while heartfelt, were delivered with a mix of despair, hopelessness, and denial.

[szlamany]

Maybe they had future plans to detect LEAP YEARS and return 366.

That makes the reason for the DATE parameters legit.

[szlamany]

The more I look at it the more I like it. That SYSTEMID - does it indicate what "client" is using the application? If that is the case then this function abstracts away from each query the "determination" of duration.

They thought ahead about all the values that might go into determining the duration.

And then coded for the simple state you are in now.

[passel]

Why do you create any function?
1. Allow self documentation of the code.
2. Reduce repetitious logic scattered throughout the code.
3. Allow modifying the logic in one place, rather than having to track down all the places one could have implemented the logic.
4. Testable code.

The user could have specified a literal 365 multiple places in the code where one might call the function with the IsAnnual flag is set, or set it some other literal or calculated value where the duration would be passed and the IsAnnual flag not be set, but what the value represents at each place may not be immediately apparent.
This is also assuming the function is being called with literal values where used. The values may be coming from another source, data driven, message driven, transaction queue, etc.

I've written code where I first write out the logic as a series of statements of what needs to be done. I then put underscores in place of the spaces, and turn them into sub or function calls. Now the process should be easy to follow as the series of sub and function calls explaining in their name what they are doing.

Then, it is just a matter of filling in the content of each sub or function to do what its name says it is doing. Some of those functions could end up being one liners, so technically would be more efficient to just put the line in the code rather than use a sub or function for it, but the "aliasing" of the operation with a descriptive method name in its place can be worth it for maintainability.
Snippet of code from a card game program as an example.

        'The cards must all be sequential and same suit to be dragged here.
        'Only kings can be moved to an empty normal stack
        'Card dragged must follow suit of card dropped on, i.e. 2 of hearts on 3 of hearts

        If To_Stack <> From_Stack Then    'don't need to process if dropped on the same stack
          If Dragged_Cards_are_Sequential_and_the_Same_Suit() Then
            If The_ToStack_is_Empty_and_The_Dragged_Card_is_a_King() Or _
               The_Last_Card_of_the_ToStack_is_One_Greater_than_the_Dragged_Card() Then

              Update_History() 'save move for Undo
              link_Card()      'Link the Card(s) to the Normal Stack dropped on
            End If
          End If
        End If
        RePosition_Cards()              'Draw cards in their new location
      End If
      Send_any_Free_Cards_Home()  'See if any cards are now free to go home

[szlamany]

This thread does NOT belong in Chit chat!

[TysonLPrice]

Along the lines of:

3. Allow modifying the logic in one place, rather than having to track down all the places one could have implemented the logic.

If it is needed all over the place this would make it so the binaries don't need recompiled/built and deployed again for leap year.

I worked on an old mainframe payroll system that did things like that with copybooks. It was just their standard...no matter how lame it might seem at times.

[FunkyDexter]

You guys do get this is in SQL right? I realise now that I never specified this and it might not be obvious given that it doesn't interact with the database in any way. I thought it would be inferable from the syntax but, if not, my bad.

The reason this is important is that functional decomposition is not a good thing in SQL. It interferes with the engines ability to build a query plan and anything using this function will inevitably be forced into some form of RBAR loop. That will be horrendous for performance if this is ever referenced in a filter (eg a Where or Join) and will be merely horrible if referenced in a large Select.

This function could be expressed in the consumer as:-

Select Case When DS.IsAnnual then 365 Else DS.StoredDuration End
From MyDataSet DS

...which is trivial and not really requiring of decomposition from a maintainability point of view.

So the obvious madness of the function is in the general weirdness of parameters that aren't used and superfluous variables. The slightly less obvious madness is in the fact that someone created code in the database which never interacts with the database. And the subtle madness is in the fact that it shouldn't bloody well exist at all.

Like I said, multi-layered.

[Shaggy Hiker]

Yeah, I saw that it was SQL, but I was thinking that it would be called from non-SQL code, more like a stored procedure. I still feel that it shows no more than the sad detritus of thwarted ambition, even if the ambition was misguided.

[szlamany]

Totally disagree with the both of.

First of all, speed in SQL is not everything.

Second, imagine you have ALL of your business logic in the database as SPROCS. This is the equivalent of a function on VB that performs a simple task - return the number of days. That determination might need the year (as in leap year = 366) and also might use the START and END dates. Even if you start your application simple - and have it always return 365 (as in no client as has for leap year specific numbers yet) and you offer an OVERRIDE STORED DURATION field for the user to load for anyone who is there less than a year (so no need today for using the START and END dates).

Here is my function in my payroll system for showing a "SALARY" figure for someone. It certainly did NOT start out this complicated - but over the past 20 years it's grown and grown and grown.

-- @Opt values are as follows:
-- 1 SDI25
-- 2 Rate Type
-- 3 Annual Rate Type
-- 4 Rate Type / Force hourly return
-- 5 Top Step
-- 6 SD debug
-- 7 Step Amt
-- 8 Annual Amt
-- 9 Step Amt / Force hourly return
-- 10 OVT
-- 11 Annual as formatted money
--
-- +100 to use current date - future-cast - instead of 7/1 backdate for 25 year logic
--
-- bit 256 to use Desc access to salary table
CREATE FUNCTION dbo.GetSalaryType_F (@Opt int, @Yr int, @UnionCode varchar(2), @Grade varchar(4), @Step varchar(3)
	, @Hours money, @HoldStep varchar(1), @CDL varchar(1), @UnitType varchar(1)
	, @PayGroup varchar(4), @StartDate datetime, @MasId int, @CurDate datetime)
RETURNS varchar(1000) AS  
BEGIN 
	Declare @ST varchar(1000)

	Set @ST=''

	Declare @CurDt int
	Set @CurDt=0

	Declare @Desc int
	Set @Desc=0

	If (@Opt & 256)<>0
	Begin
		Set @Opt = @Opt & ~ 256
		Set @Desc=1
	End

	If @Opt>=100
	Begin
		Set @CurDt=1
		Set @Opt=@Opt-100
	End

	If IsNull(@Grade,'')='' or Right(@Step,1)='M'
	Begin
		If @Opt=5
				Set @ST=''
		Else	Set @ST='* Manual *'
	End
	Else
	Begin
		If IsNull(@UnionCode,'')<>''
		Begin
			Declare @SD datetime

			Set @SD=(Select Min(StartDate) From Position_T Where MasId=@MasId and PayGroup=@PayGroup and Home='Y')

			Declare @15_20_rule int
			Set @15_20_rule=0
			If @Yr>=2015 and @UnionCode='03' Set @15_20_rule=1

			Declare @SDNow datetime
			Declare @SD15 datetime
			Declare @SD20 datetime
			Declare @SD25 datetime

/*			If IsNull(@Yr,0)>1900
			Begin
				Set @SDNow=Cast(@Yr-1 as varchar(4))+'-07-01'
				Set @SD25=Cast((@Yr-1)-25 as varchar(4))+'-07-01'
			End*/

			If @CurDt=1
					Set @SDNow=@StartDate--Cast(Convert(varchar(10),GetDate(),101) as datetime)
			Else	If IsNull(@Yr,0)>1900 Set @SDNow=Cast(@Yr-1 as varchar(4))+'-07-01'

			Declare @PD datetime
			Declare @SD15I int
			Declare @SD20I int
			Declare @SD25I int

			Set @SD15I=0
			Set @SD20I=0
			Set @SD25I=0

			Set @PD=(Select Top 1 PromotionDate From PersonStat_T Where MasId=@MasId Order by Seq Desc)

			If @15_20_rule=1
			Begin
				If @PD is not null
				Begin
					Set @SD15=DateAdd(yyyy,-15,@PD)
					Set @SD20=DateAdd(yyyy,-20,@PD)
					If @PD<'2015-07-01'
					Begin
						Set @SD20I=Case When @PD<=@SDNow Then 1 Else 0 End
					End
					Else
					Begin
						Set @SD15I=Case When @PD<=@SDNow Then 1 Else 0 End
						Set @SD20I=Case When DateAdd(yyyy,5,@PD)<=@SDNow Then 1 Else 0 End
					End
				End
				Else
				Begin
					Set @SD15=DateAdd(yyyy,-15,@SDNow)
					Set @SD15I=Case When @SD<=@SD15 Then 1 Else 0 End
					Set @SD20=DateAdd(yyyy,-20,@SDNow)
					Set @SD20I=Case When @SD<=@SD20 Then 1 Else 0 End
				End
				If @SD20I<>0 Set @SD15I=0
			End
			Else
			Begin
				If @PD is not null
				Begin
					Set @SD25=DateAdd(yyyy,-25,@PD)
					Set @SD25I=Case When @PD<=@SDNow Then 1 Else 0 End
				End
				Else
				Begin
					Set @SD25=DateAdd(yyyy,-25,@SDNow)
					Set @SD25I=Case When @SD<=@SD25 Then 1 Else 0 End
				End
			End
			--If @PD is not null
			--		Set @SD25I=Case When @PD<=@SDNow Then 1 Else 0 End
			--Else	Set @SD25I=Case When @SD<=@SD25 Then 1 Else 0 End

			If @Opt=6
			Begin
				Set @ST='PD='+IsNull(Convert(varchar(10),@PD,101),'null')
						+' SDNow='+IsNull(Convert(varchar(10),@SDNow,101),'null')
						+' SD25='+IsNull(Convert(varchar(10),@SD25,101),'null')
						+' SD20='+IsNull(Convert(varchar(10),@SD20,101),'null')
						+' SD15='+IsNull(Convert(varchar(10),@SD15,101),'null')
						+' SD='+IsNull(Convert(varchar(10),@SD,101),'null')
						+' SD25I='+IsNull(Cast(@SD25I as varchar(10)),'null')
						+' SD20I='+IsNull(Cast(@SD20I as varchar(10)),'null')
						+' SD15I='+IsNull(Cast(@SD15I as varchar(10)),'null')
				Return @ST
			End

			Declare @SFY int
			Set @SFY=@Yr

			If Not Exists(Select * From SalaryRate_T SR Where FiscalYr=@Yr and SR.UnionCode=@UnionCode)
			Begin
				Set @SFY=(Select Max(FiscalYr) From SalaryRate_T SR Where FiscalYr<@Yr and SR.UnionCode=@UnionCode)
				Set @Desc=0
			End

			If @SD25I=1
			Begin
				--If Not Exists(Select * From SalaryRateSort_T Where UnionCode=@UnionCode and RuleCd='ANNS') Set @SD25I=0
				If Not Exists(Select * From SalaryRate_T Where FiscalYr=@SFY/*@Yr*/ and UnionCode=@UnionCode and Grade=@Grade
									and RateType in (Select RateType From SalaryRateSort_T
													Where UnionCode=@UnionCode and RuleCd='ANNS')) Set @SD25I=0
			End

			If @Opt=1
			Begin
				If @15_20_rule=1
				Begin
					Set @ST='15.'+Cast(@SD15I as varchar(100))+' 20.'+Cast(@SD20I as varchar(100))
				End
				Else
				Begin
					Set @ST=@SD25I
				End
				Return @ST
			End

			If @UnitType='H' or @Opt in (4,9)
					Set @ST='HRL'
			Else	If @UnitType='D'
							Set @ST='DLY'
				Else	If @UnitType='W'
								Set @ST='WKL'
						Else	If @UnitType='B'
										Set @ST='BWK'

			If @Opt=10 Set @ST='OVT'

			If @CDL='Y' Set @ST=@ST+'CDL'
			If @CDL='M'
			Begin
				Set @ST=@ST+'M'
			End
			Else
			Begin
				If @SD15I=1 Set @ST=@ST+'S'
				If @SD20I=1 Set @ST=@ST+'X'
				If @SD25I=1 Set @ST=@ST+'S'
			End

			Declare @ANN varchar(10)
			Set @ANN='ANN'
			If @CDL='Y' Set @ANN=@ANN+'CDL'
			If @CDL='M'
			Begin
				Set @ANN=@ANN+'M'
			End
			Else
			Begin
				If @SD15I=1 Set @ANN=@ANN+'S'
				If @SD20I=1 Set @ANN=@ANN+'X'
				If @SD25I=1 Set @ANN=@ANN+'S'
			End
			Declare @RT varchar(3)
			If @15_20_rule=1
			Begin
				Set @RT=(Select Top 1 RateType From SalaryRateSort_T Where UnionCode=@UnionCode and RuleCd=@ST and Hours=@Hours and RateType not like '%25')
				If @RT is null
					Set @RT=(Select Top 1 RateType From SalaryRateSort_T Where UnionCode=@UnionCode and RuleCd=@ST and RateType not like '%25')
			End
			Else
			Begin
				Set @RT=(Select Top 1 RateType From SalaryRateSort_T Where UnionCode=@UnionCode and RuleCd=@ST and Hours=@Hours and RateType not like '%20' and RateType not like '%15')
				If @RT is null
					Set @RT=(Select Top 1 RateType From SalaryRateSort_T Where UnionCode=@UnionCode and RuleCd=@ST and RateType not like '%20' and RateType not like '%15')
			End

			If @Opt=10 Set @RT='OVT'

			If @Opt in (2,4)
			Begin
				Set @ST=@RT
				Return @ST
			End

			Declare @ART varchar(3)
			Set @ART=(Select Top 1 RateType From SalaryRateSort_T Where UnionCode=@UnionCode and RuleCd=@ANN and Hours=@Hours)
			If @ART is null
				Set @ART=(Select Top 1 RateType From SalaryRateSort_T Where UnionCode=@UnionCode and RuleCd=@ANN)

			If @Opt=3
			Begin
				Set @ST=@ART
				Return @ST
			End

	--		If @RT is null and @SD25I=1
	--		Begin
	--			Set @ST=Left(@ST,Len(@ST)-1)
	--			Set @RT=(Select Top 1 RateType From SalaryRateSort_T Where UnionCode=@UnionCode and RuleCd=@ST)
	--		End

			Set @ST=@ST+IsNull(' ('+@RT+')','')

			Set @Step=Replace(@Step,'*','')
			Set @Step=Replace(@Step,'+','')
			Set @Step=Replace(@Step,'X','')
			Set @Step=Replace(@Step,'U','')
			Set @Step=Replace(@Step,'C','')

			If IsNumeric(@Step)=0 Set @Step=''

			If @Opt=5
			Begin
				Set @ST=(Select Top 1 Case When IsNull(Step15,0)<>0 Then '15'
											When IsNull(Step14,0)<>0 Then '14'
											When IsNull(Step13,0)<>0 Then '13'
											When IsNull(Step12,0)<>0 Then '12'
											When IsNull(Step11,0)<>0 Then '11'
											When IsNull(Step10,0)<>0 Then '10'
											When IsNull(Step09,0)<>0 Then '09'
											When IsNull(Step08,0)<>0 Then '08'
											When IsNull(Step07,0)<>0 Then '07'
											When IsNull(Step06,0)<>0 Then '06'
											When IsNull(Step05,0)<>0 Then '05'
											When IsNull(Step04,0)<>0 Then '04'
											When IsNull(Step03,0)<>0 Then '03'
											When IsNull(Step02,0)<>0 Then '02'
											When IsNull(Step01,0)<>0 Then '01'
											Else '' End
										From SalaryRate_T SR
										Left join SalaryRateSort_T SO on SO.UnionCode=SR.UnionCode and SO.RateType=SR.RateType
										Where FiscalYr=@SFY/*@Yr*/ and SR.UnionCode=@UnionCode and ((SR.StartDate<=@StartDate and @Desc<>1)
																or (@StartDate<=SR.StartDate and @Desc=1))
																and SR.Grade=@Grade and SR.RateType=@RT
																and (SO.Hours=0 or SO.Hours=@Hours)
										Order by Case When @Desc=1 Then 999999999 - Convert(varchar(8),StartDate,112) Else Convert(varchar(8),StartDate,112) End /*Desc*/)
				Return @ST
			End

			Declare @StepAmt varchar(16)
			Set @StepAmt=Cast((Select Top 1 Case @Step When 1 Then Step01 
																	When 2 Then Step02
																	When 3 Then Step03
																	When 4 Then Step04
																	When 5 Then Step05
																	When 6 Then Step06
																	When 7 Then Step07
																	When 8 Then Step08
																	When 9 Then Step09
																	When 10 Then Step10 
																	When 11 Then Step11 
																	When 12 Then Step12 
																	When 13 Then Step13 
																	When 14 Then Step14
																	Else Step15 End 
										From SalaryRate_T SR
										Left join SalaryRateSort_T SO on SO.UnionCode=SR.UnionCode and SO.RateType=SR.RateType
										Where FiscalYr=@SFY/*@Yr*/ and SR.UnionCode=@UnionCode and ((SR.StartDate<=@StartDate and @Desc<>1)
																or (@StartDate<=SR.StartDate and @Desc=1))
																and SR.Grade=@Grade and SR.RateType=@RT
																and (SO.Hours=0 or SO.Hours=@Hours)
										Order by Case When @Desc=1 Then 999999999 - Convert(varchar(8),StartDate,112) Else Convert(varchar(8),StartDate,112) End /*Desc*/) as varchar(16))

			If @Opt in (7,9,10)
			Begin
				Set @ST=@StepAmt
				Return @ST
			End

			Set @ST=IsNull(@StepAmt+' ','* n/s ')+@ST

			Declare @AnnAmt varchar(100)
			Set @AnnAmt=(Select Top 1 Cast(Case @Step When 1 Then Step01 
																	When 2 Then Step02
																	When 3 Then Step03
																	When 4 Then Step04
																	When 5 Then Step05
																	When 6 Then Step06
																	When 7 Then Step07
																	When 8 Then Step08
																	When 9 Then Step09
																	When 10 Then Step10 
																	When 11 Then Step11 
																	When 12 Then Step12 
																	When 13 Then Step13 
																	When 14 Then Step14
																	Else Step15 End as varchar(16))+Case When @Opt<>11 Then ' FY'+Cast(SR.FiscalYr as varchar(4)) + ' on ' + Convert(varchar(10),SR.StartDate,101) Else '' End
																	--+Case When @Desc=1 Then  Else '' End
										From SalaryRate_T SR
										Left join SalaryRateSort_T SO on SO.UnionCode=SR.UnionCode and SO.RateType=SR.RateType
										Where FiscalYr=@SFY/*@Yr*/ and SR.UnionCode=@UnionCode and ((SR.StartDate<=@StartDate and @Desc<>1)
																or (@StartDate<=SR.StartDate and @Desc=1))
																and SR.Grade=@Grade and SR.RateType=@ART
																and (SO.Hours=0 or SO.Hours=@Hours)
										Order by Case When @Desc=1 Then 999999999 - Convert(varchar(8),StartDate,112) Else Convert(varchar(8),StartDate,112) End /*Desc*/)

			If @Opt in (8,11)
			Begin
				Set @ST=@AnnAmt
				Return @ST
			End

			Set @ST=@ST+IsNull(' ('+@AnnAmt+')','')
		End
	End

	Return @ST
/*
select 
		dbo.GetSalaryType_F(101,PO.FiscalYr
								,PO.UnionCode,PO.Grade,PO.Step,PO.Hours,PO.HoldStep,PO.CDL,PO.UnitType,PO.PayGroup,PO.StartDate,PO.MasId,GetDate())
									"SDI25"
		,dbo.GetSalaryType_F(106,PO.FiscalYr
								,PO.UnionCode,PO.Grade,PO.Step,PO.Hours,PO.HoldStep,PO.CDL,PO.UnitType,PO.PayGroup,PO.StartDate,PO.MasId,GetDate())
									"SDI25"
		,dbo.GetSalaryType_F(100,PO.FiscalYr
								,PO.UnionCode,PO.Grade,PO.Step,PO.Hours,PO.HoldStep,PO.CDL,PO.UnitType,PO.PayGroup,PO.StartDate,PO.MasId,GetDate())
									"Salary Type                              "
,po.*
from position_T po where masid in (1951)--,7699)
*/
END
GO

For testing I can call it like this:

select 
	dbo.GetSalaryType_F(101,PO.FiscalYr
		,PO.UnionCode,PO.Grade,PO.Step,PO.Hours,PO.HoldStep,PO.CDL,PO.UnitType,PO.PayGroup,PO.StartDate,PO.MasId,GetDate())
			"SDI25"
	,dbo.GetSalaryType_F(106,PO.FiscalYr
		,PO.UnionCode,PO.Grade,PO.Step,PO.Hours,PO.HoldStep,PO.CDL,PO.UnitType,PO.PayGroup,PO.StartDate,PO.MasId,GetDate())
			"SDI25"
	,dbo.GetSalaryType_F(100,PO.FiscalYr
		,PO.UnionCode,PO.Grade,PO.Step,PO.Hours,PO.HoldStep,PO.CDL,PO.UnitType,PO.PayGroup,PO.StartDate,PO.MasId,GetDate())
			"Salary Type                              "
from position_T po where masid =6111

and that returns (formatted SALARY TYPE info I put on the second line of each row - horizontal scrolling got messed up here).

SDI25	SDI25											Salary Type                              
1	PD=06/21/2001 SDNow=07/01/2010 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		2284.80 WKLS (W25) (119135.93 FY2011 on 07/01/2010)
1	PD=06/21/2001 SDNow=07/01/2011 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		2289.95 WKLS (W25) (119731.61 FY2012 on 06/30/2011)
1	PD=06/21/2001 SDNow=07/01/2012 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		2342.15 WKLS (W25) (122126.25 FY2013 on 07/01/2012)
1	PD=06/21/2001 SDNow=07/01/2013 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		2388.99 WKLS (W25) (124568.77 FY2014 on 07/01/2013)
1	PD=06/21/2001 SDNow=07/01/2014 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		2436.77 WKLS (W25) (127060.14 FY2015 on 07/01/2014)
1	PD=06/21/2001 SDNow=09/08/2014 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		0.00 WKLS (W25) (0.00 FY2015 on 07/01/2014)
1	PD=06/21/2001 SDNow=07/01/2015 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		3124.56 WKLS (W25) (163370.01 FY2016 on 07/01/2015)
1	PD=06/21/2001 SDNow=11/20/2015 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		3338.21 WKLS (W25) (174540.61 FY2016 on 07/01/2015)
1	PD=06/21/2001 SDNow=07/01/2016 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		3431.04 WKLS (W25) (178904.12 FY2017 on 07/01/2016)
1	PD=06/21/2001 SDNow=07/01/2017 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
	3516.81 WKLS (W25) (183376.72 FY2018 on 07/01/2017)
1	PD=06/21/2001 SDNow=07/01/2018 SD25=06/21/1976 SD20=null SD15=null SD=06/21/1976 SD25I=1 SD20I=0 SD15I=0	
		3674.73 WKLM (WMS) (191610.87 FY2019 on 07/01/2018)

[FunkyDexter]

speed in SQL is not everything

It's the only thing. Damnit, I just can't resist a sound bite.

You're right, it's not everything. However, it's way more important in the database that it is in, say, a client app. Maintainability is important in both and decomposition, code re-use etc. aid that. But the increased importance of performance shifts the tipping point at which we should pitch for performance before maintainability radically.

In the example code you've given I think I would agree with you and shift it out to a function. The decision driver for me is a combination of the complexity of the algorithm, the clarity of intent (sql queries don't self document well) and the number of places it's used in the system. The complexity of your example sits quite far beyond the point at which it benefits from being abstracted into a function and I'm guess it appears multiple times which supports the argument further.

But the example I found is trivial. It's a simple case statement based on two fields and it's intent is clear at a glance. There is really no argument for sacrificing performance for maintainability in this case.