Clean up UDF -- really ugly ....

S

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
 

Excel Facts

Square and cube roots
The =SQRT(25) is a square root. For a cube root, use =125^(1/3). For a fourth root, use =625^(1/4).
Sort by date Sort by votes
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
 
Upvote 0
Fluff said:
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
Click to expand...
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:
Upvote 0
Simply put the word Not in front
Code: 
ElseIf Not UBound(Filt
 
Upvote 0
Fluff said:
Simply put the word Not in front
Code: 
ElseIf Not UBound(Filt
Click to expand...

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
 
Upvote 0
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:
Upvote 0
Why test for RD? You want Reps =TM regardless of the value of RD
 
Upvote 0
Fluff said:
Why test for RD? You want Reps =TM regardless of the value of RD
Click to expand...

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
 
Upvote 0
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
 
Upvote 0
You must log in or register to reply here.

Similar threads

L
  • Locked
  • Question Question
Help Cleaning Up Code
Replies
2
Views
600
RoryA
RoryA
A
  • Solved
Calendar form cancel button passing data to worksheet
Replies
7
Views
1K
AndyEd
A
Agnarr
  • Question Question
VBA problem with creating new sheets
Replies
0
Views
832
Agnarr
Agnarr
A
  • Solved
Calendar issue
Replies
1
Views
2K
AndyEd
A
S
  • Question Question
Help on complicated VBA code
Replies
3
Views
756
superfb
S

Forum statistics

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

Share this page

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