Clean up UDF -- really ugly ....

spydey

Active Member
Joined
Sep 19, 2017
Messages
306
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
 

Some videos you may like

Excel Facts

Return population for a City
If you have a list of cities in A2:A100, use Data, Geography. Then =A2.Population and copy down.

Fluff

MrExcel MVP, Moderator
Joined
Jun 12, 2014
Messages
46,340
Office Version
  1. 365
Platform
  1. Windows
How about
Code:
Function Reps(TD, ST, RefD, FLD, RDD)
Dim Ary As Variant
Ary = Array("CA", "MO", "NV", "MA", "AL", "AZ", "GA", "MS", "MD", "NC", "NJ", "IL", "PA", "OR", "FL")

If Year(RefD) <= 2014 Then
   Reps = "TM 0"
ElseIf UBound(Filter(Ary, ST, True, vbTextCompare)) >= 0 And FLD <> "" And RDD > Now() Then
   Reps = "CTS"
ElseIf (TD >= 0 And TD <= 13) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 1"
ElseIf (TD >= 14 And TD <= 15) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 2"
ElseIf (TD >= 16 And TD <= 31) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 3"
ElseIf (TD >= 32 And TD <= 43) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 4"
ElseIf (TD >= 44 And TD <= 53) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 5"
ElseIf (TD >= 54 And TD <= 67) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 6"
ElseIf (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 Function
 

spydey

Active Member
Joined
Sep 19, 2017
Messages
306
Office Version
  1. 2013
Platform
  1. Windows
How about
Code:
Function Reps(TD, ST, RefD, FLD, RDD)
Dim Ary As Variant
Ary = Array("CA", "MO", "NV", "MA", "AL", "AZ", "GA", "MS", "MD", "NC", "NJ", "IL", "PA", "OR", "FL")

If Year(RefD) <= 2014 Then
   Reps = "TM 0"
ElseIf UBound(Filter(Ary, ST, True, vbTextCompare)) >= 0 And FLD <> "" And RDD > Now() Then
   Reps = "CTS"
ElseIf (TD >= 0 And TD <= 13) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 1"
ElseIf (TD >= 14 And TD <= 15) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 2"
ElseIf (TD >= 16 And TD <= 31) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 3"
ElseIf (TD >= 32 And TD <= 43) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 4"
ElseIf (TD >= 44 And TD <= 53) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 5"
ElseIf (TD >= 54 And TD <= 67) And Year(RefD) > 2014 And FLD <> "" And RDD <= Now() Then
   Reps = "TM 6"
ElseIf (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 Function
Fluff,

Thank you once again for your input!!

I was looking at the code and a thought popped into my head. What if I wanted to exclude all of the items in the Array?

Currently with this line:

Code:
[COLOR=#333333]UBound(Filter(Ary, ST, True, vbTextCompare)) >= 0[/COLOR]

we are stating if the ST matches any of the array components, right? What if I wanted to exclude the array components, would I simple change the TRUE to FALSE?

Code:
[COLOR=#333333]UBound(Filter(Ary, ST, [B]FALSE[/B], vbTextCompare)) >= 0[/COLOR]

When I gave it a go, it didn't give me the expected output.

Basically, I want to change the code to something like this:

Code:
Function Reps(TD, ST, RefD, FLD, RDD)Dim Ary As Variant
Ary = Array("CA", "MO", "NV", "MA", "AL", "AZ", "GA", "MS", "MD", "NC", "NJ", "IL", "PA", "OR", "FL")


If Year(RefD) <= 2014 Then
   Reps = "TM 0"
ElseIf UBound(Filter(Ary, ST, True, vbTextCompare)) >= 0 And FLD <> "" And UBound(Filter(Ary, ST, FALSE, vbTextCompare)) >= 0 Then
   Reps = "CTS"
ElseIf (TD >= 0 And TD <= 13) And Year(RefD) > 2014 And FLD <> "" And UBound(Filter(Ary, ST, FALSE, vbTextCompare)) >= 0) Then
   Reps = "TM 1"
ElseIf (TD >= 14 And TD <= 15) And Year(RefD) > 2014 And FLD <> "" And UBound(Filter(Ary, ST, FALSE, vbTextCompare)) >= 0 Then
   Reps = "TM 2"
ElseIf (TD >= 16 And TD <= 31) And Year(RefD) > 2014 And FLD <> "" And UBound(Filter(Ary, ST, FALSE, vbTextCompare)) >= 0 Then
   Reps = "TM 3"
ElseIf (TD >= 32 And TD <= 43) And Year(RefD) > 2014 And FLD <> "" And UBound(Filter(Ary, ST, FALSE, vbTextCompare)) >= 0 Then
   Reps = "TM 4"
ElseIf (TD >= 44 And TD <= 53) And Year(RefD) > 2014 And FLD <> "" And UBound(Filter(Ary, ST, FALSE, vbTextCompare)) >= 0 Then
   Reps = "TM 5"
ElseIf (TD >= 54 And TD <= 67) And Year(RefD) > 2014 And FLD <> "" And UBound(Filter(Ary, ST, FALSE, vbTextCompare)) >= 0 Then
   Reps = "TM 6"
ElseIf (TD >= 68 And TD <= 99) And Year(RefD) > 2014 And FLD <> "" And UBound(Filter(Ary, ST, FALSE, vbTextCompare)) >= 0 Then
   Reps = "TM 7"
Else
   Reps = "N/A"
End If
End Function

So Reps needs to equal TM 1 - 7 if the TD, RefD, & FLD criteria are met, as well as the ST cannot be one of the items in the array.

Any thoughts?

-Spydey
 
Last edited:

Fluff

MrExcel MVP, Moderator
Joined
Jun 12, 2014
Messages
46,340
Office Version
  1. 365
Platform
  1. Windows

ADVERTISEMENT

Simply put the word Not in front
Code:
ElseIf Not UBound(Filt
 

spydey

Active Member
Joined
Sep 19, 2017
Messages
306
Office Version
  1. 2013
Platform
  1. Windows
Simply put the word Not in front
Code:
ElseIf Not UBound(Filt

WOW!!! That is so much easier than I thought it would be. hahahahaha, somethings the simple things elude me .....

Thanks for your help and willingness to assist. I appreciate it!

-Spydey
 

spydey

Active Member
Joined
Sep 19, 2017
Messages
306
Office Version
  1. 2013
Platform
  1. Windows

ADVERTISEMENT

Fluff,

Again, thanks for your help.

I was looking at the code and realized I need to meet two different scenarios for each TM.

Basically, after the TM 0 & CTS, once the RefD & FLD criteria are met, if the RD is less than or equal to today (regardless of the ST), then assign TM to Reps, else if the RD is greater than today but is NOT one of the ST in the array, then assign to TM.

I have two lines of code for that, but was thinking there as to be a better way, a more efficient and compact way to do it.

Here is the code:

Code:
Function Reps(TD, ST, RefD, FLD, RDD)Dim Ary As Variant
Ary = Array("CA", "MO", "NV", "MA", "AL", "AZ", "GA", "MS", "MD", "NC", "NJ", "IL", "PA", "OR", "FL")


If Year(RefD) <= 2014 And RefD <> "" Then
   Reps = "TM 0"
ElseIf UBound(Filter(Ary, ST, True, vbTextCompare)) >= 0 And FLD <> "" And RDD > Now() Then
   Reps = "CTS"
ElseIf Year(RefD) > 2014 And FLD <> "" And RD <= Now() Then
   Reps = "TM"
ElseIf Year(RefD) > 2014 And FLD <> "" And RD > Now() And Not UBound(Filter(Ary, ST, True, vbTextCompare)) >= 0 Then
   Reps = "TM"
Else
Reps="N/A"
End If
End Function

One does not take into account the ST and associated array, whereas the other does; the difference being the date of the RD being less than, equal to, or greater than today (Now).

Do I have it written correctly and/or is there is more efficient way to do this so that I don't have to use 2 lines ..... ????

So basically, it seems I will need 2 lines of code per TM, as per my original TM 1- 7, which doubles the lines of code I was going to use. Yuck!

-Spydey
 
Last edited:

Fluff

MrExcel MVP, Moderator
Joined
Jun 12, 2014
Messages
46,340
Office Version
  1. 365
Platform
  1. Windows
Why test for RD? You want Reps =TM regardless of the value of RD
 

spydey

Active Member
Joined
Sep 19, 2017
Messages
306
Office Version
  1. 2013
Platform
  1. Windows
Why test for RD? You want Reps =TM regardless of the value of RD

After TM 0 & CTS, Reps = TM under two scenarios:

If RD is less than or equal to today's date, regardless of the ST (can include the ST from the array)

or

If RD is greater than today's date BUT only for ST that are NOT in the array (can not include ST from the array)



I believe I see what you are getting at by why need to test for RD if it is covering the whole spectrum of Less than, equal to, greater than, right??????
You're stating that regardless of the RD value, after TM 0 & CTS, Reps will equal TM, assuming the RefD and FLD criteria are met.

So, because we previously isolated via the TM 0 & CTS code, the items which would NOT be TM, we can then code accordingly, and thus get rid of the extra coding, right? Did I understand that correctly?

Thanks for the insight Fluff!! You always come through. I appreciate it very much.

-Spydey
 

Fluff

MrExcel MVP, Moderator
Joined
Jun 12, 2014
Messages
46,340
Office Version
  1. 365
Platform
  1. Windows
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
 

Watch MrExcel Video

Forum statistics

Threads
1,109,020
Messages
5,526,297
Members
409,694
Latest member
bastos21

This Week's Hot Topics

Top