Clean up UDF -- really ugly ....

spydey

Active Member
Joined
Sep 19, 2017
Messages
314
Office Version
  1. 2013
Platform
  1. Windows
I have a UDF that I wrote .... nothing fancy, but I could sure use some help cleaning up. It is very basic and I am not 100% sure where or how I should clean it up so that it is easier on the eyes, easier to follow (doesn't make one cringe just looking at it), better coding etiquette/form/procedure (???), faster, etc.

Any insight anyone can give, would be very helpful, even just pointing me in the right direction.

I am using a lot of IF THEN ELSE statements, 9 of them actually, and I think it is just clunky and bad form ......

What do you think?

TD = 2 digits, ranging from 00 - 99
ST = State abreviation
RefD = Date, ranging from 01/01/2000 - TODAY
FLD = Date, ranging from BLANK - YESTERDAY
RDD = Date, ranging from BLANK - FUTURE DATE


Code:
Function Reps(TD, ST, RefD, FLD, RDD)

If Year(RefD) <= 2014 Then
    
Reps = "TM 0"
    
Else
        
If (ST = "CA" Or ST = "MO" Or ST = "NV" Or ST = "MA" Or ST = "AL" Or ST = "AZ" Or ST = "GA" Or ST = "MS" Or ST = "MD" Or ST = "NC" Or ST = "NJ" Or ST = "IL" Or ST = "PA" Or ST = "OR" Or ST = "FL") And FLD <> "" And RDD > Now() Then


Reps = "CTS"


Else


If (TD >= 0 And TD <= 13) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then


Reps = "TM 1"


Else


If (TD >= 14 And TD <= 15) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then


Reps = "TM 2"


Else


If (TD >= 16 And TD <= 31) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then


Reps = "TM 3"


Else


If (TD >= 32 And TD <= 43) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then


Reps = "TM 4"


Else


If (TD >= 44 And TD <= 53) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then


Reps = "TM 5"


Else


If (TD >= 54 And TD <= 67) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then


Reps = "TM 6"


Else


If (TD >= 68 And TD <= 99) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then


Reps = "TM 7"


Else


Reps = "N/A"


End If
End If
End If
End If
End If
End If
End If
End If
End If


End Function


Thanks in advance!

-Spydey
 
You've understood me correctly, but looking at it again, I misunderstood what you wanted.
So there's nothing wrong with what you posted in post#7

Interestingly, I think that we happen to both be correct. hahahaha

Due to the fact that we already isolated the population of items where RD > Now() for a specific set of ST, as found in the arry, and that is getting tested first in the code for the CTS population, then logically once we get to testing for the TM population, whatever is left over with a RD > Now() wouldn't have a corresponding ST from the array as it would have already been assigned out to CTS.

So taking that into consideration, I ended up back at basically the initial code you wrote (thank you again for that) with a minor change. I simply removed the AND RD <= NOW() statement at the end of the code for each TM (TM 1 - 7).

Comparing with results while using the two lines of code per TM that I wrote, I get the same exact output.

I don't think I would have arrived at this conclusion had you not pointed out that there really was no need to test for RD when we were covering the whole gambit (<,=,>).

So thank you Fluff for your input, patience, willingness to direct me, and for your guidance. I find it very helpful.

-Spydey
 
Upvote 0

Excel Facts

Remove leading & trailing spaces
Save as CSV to remove all leading and trailing spaces. It is faster than using TRIM().
Glad to help & thanks for the feedback
 
Upvote 0
If I am not mistaken, this (untested) function should duplicate the results of the code you posted in Message #1 ...
Code:
[table="width: 500"]
[tr]
	[td]Function Reps(TD, ST, RefD, FLD, RDD)
  If Year(RefD) <= 2014 Then
    Reps = "TM 0"
  ElseIf FLD <> "" Then
    If InStr(" CA MO NV MA AL AZ GA MS MD NC NJ IL PA OR FL ", " " & ST & " ") > 0 And RDD > Now() Then
      Reps = "CTS"
    ElseIf RDD <= Now() Then
      Reps = "TM " & Evaluate("LOOKUP(" & TD & ",{0,14,16,32,44,54,68},{1,2,3,4,5,6,7,8})")
    Else
     Reps = "N/A"
    End If
  End If
End Function[/td]
[/tr]
[/table]
 
Upvote 0

Forum statistics

Threads
1,224,820
Messages
6,181,160
Members
453,021
Latest member
Justyna P

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top