Solved How to avoid hard coding names etc

January 4, 2019 at 07:48:50
Specs: Windows 10
I have a spreadsheet where the users want to be alerted to cases about to fall into their next case age bracket. See https://www.computing.net/answers/office/how-to-find-and-copy-data-based-on-several-trigger-dates/21472.html for the original question.

I have three issues that I don't know how to, or can't seem to solve:

1) Avoiding hard coding people's names into the macro
2) Simplifying the code
3) Getting a message box to work

Avoiding hard coding people's names into the macro
There are two names who are regular team members (whose surnames I've ommitted) that own the bulk of the cases the data refers to. Their manager wants to monitor their case management and so on the "Workload" sheet, the code returns their case references about to fall into new case age brackets in columns S:X. There are on rare occasions some other case owners and so I've dealt with this by adding an "Other" section where references and case age brackets are added in columns Y:Z. The section of the worksheet with example references therefore looks like this:

     S    T     U    V    W    X     Y      Z
4      30+        90+      180+        Other
5   Jo  Laura  Jo  Laura Jo  Laura  Ref  Bracket
6  2005                 1971       2010    30+
7        1998
8        1997

I'm conscious though, that the two main names could change in the future and was wondering if in your experience you know of a better way of handling this?

Simplifying the code
The section of code underneath the comment "If new Bracket required, copy case references only from column "F"..." represents my stab at populating the layout shown above. Given I'm a novice at this, I was wondering if there's a simpler, more elegant way of achieving the same?

Getting a message box to work
There are also occasional issues with dates being input incorrectly into the source data which I'm unable to control at source. With the help of this forum I've dealt with that by copying the offending dates and case reference numbers from the "Data" worksheet into the "Workload" worksheet. I also wanted to alert the user when problematic dates have been copied and have attempted to put a message box line at the end of the code, that triggers when cell AB5 is populated. Unfortunately I just keep getting a Compile error: Expected: list separator or ) pop up. When I comment out the message box line, everything works perfectly. I've checked that there aren't any curly quotes etc but can't find anything with the syntax. Can you help with this?

Here's the code in full:

Sub DateBrackets_v3()

On Error Resume Next

'Determine last row with data in case date bracket moves on "Workload" sheet _
note using Find because some columns are longer than others in the range _
as well as empty columns
clrBrkData = Cells.Find(What:="*", After:=Sheets("Workload").Range("A1"), _
            LookAt:=xlPart, LookIn:=xlFormulas, _
            SearchOrder:=xlByRows, SearchDirection:=xlPrevious, _
            MatchCase:=False).Row

'Clear date bracket cases from "Workload" sheet, Leave Header Row
  Sheets("Workload").Range("S6:Z" & clrBrkData).ClearContents

Application.ScreenUpdating = False

'Determine Last Row with data on "Data" sheet
  lastSrcRw = Sheets("Data").Cells(Rows.Count, "F").End(xlUp).Row

'Loop through Column S, Status must be "Work in progress"
    For myDates = 2 To lastSrcRw
     If Sheets("Data").Cells(myDates, "S") = "Work in progress" Then
     
'Validate date in Column Q on "Data" sheet
       If Not IsDate(Sheets("Data").Cells(myDates, "Q")) Then
       With Sheets("Workload")
        nxtBadDt = .Cells(Rows.Count, 28).End(xlUp).Row + 1
        .Cells(nxtBadDt, "AB") = Sheets("Data").Cells(myDates, "F")
        .Cells(nxtBadDt, "AC") = Sheets("Data").Cells(myDates, "Q")
       End With
       Else
       
       
'Calculate Case Age from Column Q
      caseAge = Date - Sheets("Data").Cells(myDates, "Q")
      
      End If
           
'Assign CI Mgr name
      ciMgr = Sheets("Data").Cells(myDates, "R")
   
'Set New Bracket based on Case Age
      If caseAge < 181 Then
        If 180 - caseAge < 28 Then
           new_Bracket = "180 Plus"
        ElseIf 91 - caseAge < 14 Then
           new_Bracket = "91 to 180"
        ElseIf 30 - caseAge < 7 Then
           new_Bracket = "31 to 90"
        End If
      End If

     
 'If new Bracket required, copy case references only from column "F" _
 on "Data" sheet to the case bracket columns on the "Workload" sheet, _
 as well as the references and date brackets for any cases not belonging to the key team members
      If new_Bracket <> "" Then
        If ciMgr = "Jo" And new_Bracket = "31 to 90" Then
        With Sheets("Workload")
          nxtDstRw = .Cells(Rows.Count, 19).End(xlUp).Row + 1
         .Cells(nxtDstRw, "S") = Sheets("Data").Cells(myDates, "F")
        End With
        ElseIf ciMgr = "Laura" And new_Bracket = "31 to 90" Then
        With Sheets("Workload")
          nxtDstRw = .Cells(Rows.Count, 20).End(xlUp).Row + 1
         .Cells(nxtDstRw, "T") = Sheets("Data").Cells(myDates, "F")
        End With
        ElseIf ciMgr = "Jo" And new_Bracket = "91 to 180" Then
        With Sheets("Workload")
          nxtDstRw = .Cells(Rows.Count, 21).End(xlUp).Row + 1
         .Cells(nxtDstRw, "U") = Sheets("Data").Cells(myDates, "F")
        End With
        ElseIf ciMgr = "Laura" And new_Bracket = "91 to 180" Then
        With Sheets("Workload")
          nxtDstRw = .Cells(Rows.Count, 22).End(xlUp).Row + 1
         .Cells(nxtDstRw, "V") = Sheets("Data").Cells(myDates, "F")
        End With
        ElseIf ciMgr = "Jo" And new_Bracket = "180 Plus" Then
        With Sheets("Workload")
          nxtDstRw = .Cells(Rows.Count, 23).End(xlUp).Row + 1
         .Cells(nxtDstRw, "W") = Sheets("Data").Cells(myDates, "F")
        End With
        ElseIf ciMgr = "Laura" And new_Bracket = "180 Plus" Then
        With Sheets("Workload")
          nxtDstRw = .Cells(Rows.Count, 24).End(xlUp).Row + 1
         .Cells(nxtDstRw, "X") = Sheets("Data").Cells(myDates, "F")
        End With
        ElseIf ciMgr <> "Jo" And "Laura" Then
        With Sheets("Workload")
          nxtDstRw = .Cells(Rows.Count, 25).End(xlUp).Row + 1
          .Cells(nxtDstRw, "Y") = Sheets("Data").Cells(myDates, "F")
          .Cells(nxtDstRw, "Z") = new_Bracket
        End With
        End If
           
'Clear New Bracket flag
      new_Bracket = ""
      End If
      
     End If
     
    Next

If Sheets("Workload").Range("AB5") <> "" Then
    MsgBox ("The SOFIT database contains incorrect dates in the Date Assigned column." _
    & vbNewLine & vbNewLine & "Please review which submissions need correction and take appropriate _
    corrective action", vbOKOnly + vbExclamation, "ATTENTION!")
End If

End Sub

message edited by ScottV


See More: How to avoid hard coding names etc

Reply ↓  Report •

#1
January 4, 2019 at 10:11:28
✔ Best Answer
I'm going to answer your questions in reverse order:

Getting a message box to work

You have a couple of syntax errors in that instruction.

1 - (This one is kinda of easy)

When you use the line-continuation character (the underscore) in the middle of a text string, you need to enclose both sections of the string with the double quotes. Otherwise the compiler considers the underscore as part of the text string that started with the opening double quotes and hasn't ended yet. It throws up an error because, in reality, you split the line without using the line continuation character.

2 - (This one I would have trouble explaining clearly, so I'll just tell you how to correct it.)

While fixing the syntax error related to the line-continuation character, also remove the parenthesis.

For an explanation of why you shouldn't use the parentheses in your situation, I suggest you DAGS for info on when to use parenthesis in a MsgBox instruction and when not to. Once again, it is related to how the compiler treats the information inside the parentheses, but it's too deep of a discussion for me to get into.

Simplifying the code

I don't have a copy of your workbook to test against, so I'll just toss out some suggestions.

1 - Does this actually work for you?

ElseIf ciMgr <> "Jo" And "Laura" Then

I'm pretty sure that it needs to be more explicit. I don't think that VBA will apply the And directly to the "ciMgr <>". I think you need to use this:

ElseIf ciMgr <> "Jo" And ciMgr <> "Laura" Then

2 - I think you can use one With Sheets("Workload")...End With around that entire If...ElseIf...End If section. You are referencing the same sheet (Workload) every time, so just reference it once.

3 - I'm pretty sure that you should be able to combine each "bracket" inside a single If instead of using And. I'm not sure you'll gain much from an efficiency perspective, but it might make the code easier to read:

Combining suggestions 2 & 3, something like this should work:

        With Sheets("Workload")
           If new_Bracket = "31 to 90" Then

              If ciMgr = "Jo" then
                 nxtDstRw = .Cells(Rows.Count, 19).End(xlUp).Row + 1
                .Cells(nxtDstRw, "S") = Sheets("Data").Cells(myDates, "F")

             ElseIf  ciMgr = "Laura"
                 etc.

             End If
      
           If new_Bracket = "91 to 180" Then
         
              If ciMgr = "Jo" then
                 etc.

              ElseIf  ciMgr = "Laura"
                 etc.

             End If

        End With

Avoiding hard coding people's names into the macro

If you don't want the names in the macro, they'll have to be pulled from the workbook. Create a list of employee names somewhere in the workbook and reference the cell that contains the name you are currently interested in. After all, if you are searching specifically for data related to "Jo", "Jo" must be stored someplace so that VBA knows what data to look for. If "Jo" isn't stored within VBA someplace, it'll have to be stored within the workbook.

      If ciMgr = Sheets("Personnel").Range("A2") Then
          etc.

     ElseIf ciMgr = ciMgr = Sheets("Personnel").Range("A3")
          etc.

When Jo retires just update the Personnel sheet with the name of her replacement and the macro will use that name from then on.

I hope that helps.

How To Post Data or Code ---> Click Here Before Posting Data or VBA Code


Reply ↓  Report •

#2
January 7, 2019 at 08:24:53
Message Box
1 - Of course! I hadn't thought of that but it makes complete sense now you say it. I added in the required quotes and it's working fine now.

2 - I haven't delved into this too deeply for now, but from what I can see, because I'm not returning any values from the message box/populating any variables etc, I don't need the parentheses. I've taken them away and will look into this in more depth in the next couple of weeks.

Simplifying the code
1 -

ElseIf ciMgr <> "Jo" And "Laura" Then
was working. However, in the interests of that being lucky and benefitting from your knowledge, I've changed the code as suggested to be more explicit.

2 - You're right. One

With Sheets...
instruction was sufficient so I've removed all the unnecessary repeats

3 - I'm not sure if you meant to omit the

If new_Bracket <> ""
or just assumed I'd know I needed it? Either way, through trial and error I realised I do need that otherwise the macro gets a bit funky for want of a better word.

Avoiding hard coding people's names into the macro
Having weighed up what you said, I've decided to leave the names in the code, but put in an extra comment warning the next 'developer' (or me if it still is me) to change the names when necessary.

I added in

'Clear incorrectly formatted dates info from columns AB:AC
  Sheets("Workload").Range("AB5:AC" & clrBrkData).ClearContents

because I realised I wasn't clearing the old incorrectly formatted dates on the dashboard each time, and I couldn't figure out how to extend
'Clear date bracket cases from "Workload" sheet, Leave Header Row
  Sheets("Workload").Range("S6:Z" & clrBrkData).ClearContents

to catch that second range. That's because the headers for the two ranges look like this, so row 6 (and below) was getting cleared of the case references, but row 5 in the range AB5:AC5 wasn't:

     S    T     U    V    W    X     Y      Z       AA        AB          AC
4      30+        90+      180+        Other                  Ref    Incorrect Date
5   Jo  Laura  Jo  Laura Jo  Laura  Ref  Bracket             2015     13/12/20018
6  2005                 1971       2010    30+
7        1998
8        1997

So all is working perfectly well now. As a British subject, I've assumed the power to grant you an honorary knighthood and so henceforth you shall be known as Sir DerbyDad03. Sorry for my somewhat zany sense of humour but it gets me through the day. ;-)

Here's the finished code in full for anyone who can make use of it:

Sub DateBrackets()

On Error Resume Next

'Determine last row with data in case date bracket moves on "Workload" sheet _
note using Find because some columns are longer than others in the range _
as well as empty columns
clrBrkData = Cells.Find(What:="*", After:=Sheets("Workload").Range("A1"), _
            LookAt:=xlPart, LookIn:=xlFormulas, _
            SearchOrder:=xlByRows, SearchDirection:=xlPrevious, _
            MatchCase:=False).Row

'Clear date bracket cases from "Workload" sheet, Leave Header Row
  Sheets("Workload").Range("S6:Z" & clrBrkData).ClearContents
  
'Clear incorrectly formatted dates info from columns AB:AC. Leave Header Row.
  Sheets("Workload").Range("AB5:AC" & clrBrkData).ClearContents

Application.ScreenUpdating = False

'Determine Last Row with data on "Data" sheet
  lastSrcRw = Sheets("Data").Cells(Rows.Count, "F").End(xlUp).Row

'Loop through Column S, Status must be "Work in progress"
    For myDates = 2 To lastSrcRw
     If Sheets("Data").Cells(myDates, "S") = "Work in progress" Then
     
'Validate date in Column Q on "Data" sheet
       If Not IsDate(Sheets("Data").Cells(myDates, "Q")) Then
       With Sheets("Workload")
        nxtBadDt = .Cells(Rows.Count, 28).End(xlUp).Row + 1
        .Cells(nxtBadDt, "AB") = Sheets("Data").Cells(myDates, "F")
        .Cells(nxtBadDt, "AC") = Sheets("Data").Cells(myDates, "Q")
       End With
       Else
       
       
'Calculate Case Age from Column Q
      caseAge = Date - Sheets("Data").Cells(myDates, "Q")
      
      End If
           
'Assign CI Mgr name
      ciMgr = Sheets("Data").Cells(myDates, "R")
   
'Set New Bracket based on Case Age
      If caseAge < 181 Then
        If 180 - caseAge < 28 Then
           new_bracket = "180 Plus"
        ElseIf 91 - caseAge < 14 Then
           new_bracket = "91 to 180"
        ElseIf 30 - caseAge < 7 Then
           new_bracket = "31 to 90"
        End If
      End If

     
 'If new Bracket required, copy case references only from column "F" _
 on "Data" sheet to the case bracket columns on the "Workload" sheet, _
 as well as the references and date brackets for any cases not belonging to the key team members
 
 'Also note that if a key team member leaves then code below will need to be _
 amended to reflect the new names that need to be searched for
      If new_bracket <> "" Then
      With Sheets("Workload")
        If new_bracket = "31 to 90" Then
          If ciMgr = "Jo" Then
          nxtDstRw = .Cells(Rows.Count, 19).End(xlUp).Row + 1
         .Cells(nxtDstRw, "S") = Sheets("Data").Cells(myDates, "F")
          ElseIf ciMgr = "Laura" Then
          nxtDstRw = .Cells(Rows.Count, 20).End(xlUp).Row + 1
         .Cells(nxtDstRw, "T") = Sheets("Data").Cells(myDates, "F")
          End If
        End If
        
        If new_bracket = "91 to 180" Then
          If ciMgr = "Jo" Then
          nxtDstRw = .Cells(Rows.Count, 21).End(xlUp).Row + 1
         .Cells(nxtDstRw, "U") = Sheets("Data").Cells(myDates, "F")
          ElseIf ciMgr = "Laura" Then
          nxtDstRw = .Cells(Rows.Count, 22).End(xlUp).Row + 1
         .Cells(nxtDstRw, "V") = Sheets("Data").Cells(myDates, "F")
          End If
        End If
        
        If new_bracket = "180 Plus" Then
          If ciMgr = "Jo" Then
          nxtDstRw = .Cells(Rows.Count, 23).End(xlUp).Row + 1
         .Cells(nxtDstRw, "W") = Sheets("Data").Cells(myDates, "F")
          ElseIf ciMgr = "Laura" Then
          nxtDstRw = .Cells(Rows.Count, 24).End(xlUp).Row + 1
         .Cells(nxtDstRw, "X") = Sheets("Data").Cells(myDates, "F")
          End If
        End If
          
        If ciMgr <> "Jo" And ciMgr <> "Laura" Then
          nxtDstRw = .Cells(Rows.Count, 25).End(xlUp).Row + 1
         .Cells(nxtDstRw, "Y") = Sheets("Data").Cells(myDates, "F")
         .Cells(nxtDstRw, "Z") = new_bracket
        End If
    End With
        
           
'Clear New Bracket flag
      new_bracket = ""
      End If
    End If
      
    Next

If Sheets("Workload").Range("AB5") <> "" Then
    MsgBox "The SOFIT database contains incorrect dates in the Date Assigned column." _
    & vbNewLine & vbNewLine & "Please review which submissions need correction and take appropriate" _
    & "corrective action.", vbOKOnly + vbExclamation, "ATTENTION!"
End If

End Sub

message edited by ScottV


Reply ↓  Report •

#3
January 7, 2019 at 12:20:12
Simplifying the code

re: ElseIf ciMgr <> "Jo" And "Laura" Then was working.

My guess from across the pond is that the only reason it was "working" was because the code never evaluated the instruction. The compiler didn't complain about the syntax, but if the run-time engine ever tried to evaluate that instruction I think you would have gotten a Type Mismatch error.

The VBA And operator requires 2 or more expressions to compare. All expressions must evaluate to either True or False. ciMgr <> "Jo" is an expression that evaluates to True or False. "Laura" is not an expression, it's just a Text String.

Inside VBA, the expressions get evaluated and the results just sort of hang around waiting to be used. Suddenly the And operated comes along, grabs the first result (True) and then grabs the second result ("Laura"). Expecting to compare only the Boolean values True and False, VBA throws up a Type Mismatch error because a Text String is not the same data type as a Boolean value

Single Step (F8) through this simple macro and see what happens:

Sub TestDataType()

  If ciMgr <> "Jo" And ciMgr <> "Laura" Then
     MsgBox "Not Jo And Laura"
  End If

   If ciMgr <> "Jo" And "Laura" Then
     MsgBox "Not Jo And Laura"
   End If

End Sub

One more suggestion, which may not fit your work process, but I'll toss it out anyway: Perhaps you could use the code to "push" the user towards correcting the offending dates right away, instead of leaving it to chance.

The simplest way might be just a "hint" to get busy by sending them to the cell that contains the invalid date.

If Sheets("Workload").Range("AB5") <> "" Then

  'You present your MsgBox, the user clicks OK, and then:

  Sheets("Workload").Range("AB5").Select 

  'Now they are staring at the first invalid date

End if

How To Post Data or Code ---> Click Here Before Posting Data or VBA Code


Reply ↓  Report •

Related Solutions

#4
January 7, 2019 at 12:50:30
DerbyDad03: Suddenly the And operated comes along, grabs the first result (True) and then grabs the second result ("Laura"). Expecting to compare only the Boolean values True and False, VBA throws up a Type Mismatch error because a Text String is not the same data type as a Boolean value
It's a bit more complicated than that. And is a bitwise operator. So, ciMgr <> "Jo" is evaluated, and is converted from a Boolean to an Integer (either -1 or 0, for True or False). Next, "Laura" is converted into a number. (Can't, error suppressed by On Error Resume Next. Ignore rest of paragraph.) Then both numbers go through a binary And. Finally, the resulting number is converted to a Bool (0 = False, everything else = True) to be used by the If statement.

ScottV: [T]wo main names could change in the future and was wondering if [there's] a better way of handling this?
Is there anything in the data that denotes how these individuals should be treated? If so, use that. If not, move the name references to a single set of constants, and reference that.

ScottV: Given I'm a novice at this, I was wondering if there's a simpler, more elegant way of achieving [example output]?
I'd have to see sample input covering the various cases, and the expected output before I can say anything substantial, but I'd probably make a Function that takes a name and a bracket, and returns a list/array of the targeted cases.

How To Ask Questions The Smart Way

message edited by Razor2.3


Reply ↓  Report •

#5
January 7, 2019 at 13:06:50
Razor2.3

re:It's a bit more complicated than that. And is a bitwise operator. So, ciMgr <> "Jo" is evaluated, and is converted from a Boolean to an Integer (either -1 or 0, for True or False).

You are correct with that deeper explanation. I purposely didn't go down the the whole integer conversion path (especially the -1 part) but since you are calling me out on my explanation, I guess I should have. ;-)

I did consider the On Error Resume Next. In fact, I didn't see that until later. I meant to ask ScottV about that but forgot.

Thanks for bringing that up.

How To Post Data or Code ---> Click Here Before Posting Data or VBA Code


Reply ↓  Report •

#6
January 7, 2019 at 14:37:03
Mostly I just wanted to point out the On Error Resume Next, because any conversation about what is / is not "working" needs the tacit understanding that "working" does not mean "script does not halt with an error dialog."

How To Ask Questions The Smart Way

message edited by Razor2.3


Reply ↓  Report •

#7
January 9, 2019 at 05:46:49
Thanks Both - I feel like I've been taken down the rabbit hole! :-)

I put the On Error Resume Next line in before DerbyDad03 helped me with

If Not IsDate(Sheets("Data").Cells(myDates, "Q")) Then
in a previous thread, because I was getting type mismatch errors.

I followed some of what you're both saying but not all.

1) Are you saying I should remove the On Error Resume Next instruction? After googling, I understand that this isn't a good way of dealing with errors, and given my main issue was being caused by incorrect date inputs in the source data, I guess the need for it is dealt with by the IsDate function. Is that what you're driving at?

2) Razor2.3, forgive my lack of knowledge but I don't really know what you mean when you say "...move the name references to a single set of constants, and reference that."

3) And I definitely don't understand what to do with "...make a Function that takes a name and a bracket, and returns a list/array of the targeted cases."


Reply ↓  Report •

#8
January 9, 2019 at 09:52:13
1) On Error Resume Next is how you tell VBA, "Shut up and trust me, I know what I'm doing," and computers are gullible enough to believe you. Unless you're dealing with a situation where you'd need your VBA code to handle any errors itself, you shouldn't use it.

2) Basically a variable the code can't change. https://docs.microsoft.com/en-us/of...

3) It's about having code locate one specific person/bracket pair, and just run that for each person/bracket pair you care about. https://www.excel-easy.com/vba/func...

How To Ask Questions The Smart Way


Reply ↓  Report •

#9
January 9, 2019 at 11:08:00
Let me jump in and address item 3 above.

I may be mistaken, but I don't think that the use a "person/bracket pair" Function would help in this case. I'm not saying it couldn't be used, just that I don't think it would help. In essence what the code is doing is this:

1 - Search a specific column for a specific string ("Work in Progress")
2 - In any row that the string appears, do a Date calculation based on the date in Column Q
3 - If the date is valid and the calculation lands within 1 of 3 "date brackets", copy some data from that row (including the person's name) to Table 1 in a different worksheet and add the proper "date bracket string" to the copied data.
4 - If the date is not a valid date, copy different data (not including the person's name) from that row to Table 2 in a different worksheet.

In other words, there isn't really any "person/bracket pair" to find. The bracket (1 of 3) is determined by a date calculation and, based on that calculation, may not even exist. The "person" just sort of comes along with that bracket, assuming it is found. In other, other words, it might any one of these or more:

Jo & Bracket "91 to 180"
Laura & Bracket "91 to 180"
Jo & Bracket "180 Plus"
Bob & Bracket "180 Plus"
No bracket at all (then nothing copied from that row)
An invalid date (then different data copied to a different table)

I guess the easiest way to say all that is that there is no "person/bracket pair" until the code determines that one exists for the data in any given row.

message edited by DerbyDad03


Reply ↓  Report •

#10
January 9, 2019 at 12:12:13
As long as I'm here, I might as well jump in on Item 2 also. ;-)

Razor2.3 said " "...move the name references to a single set of constants, and reference that." and then explained that to mean "2) Basically a variable the code can't change."

Earlier on I had said "If you don't want the names in the macro, they'll have to be pulled from the workbook."

When I say "in the macro" and R2.3 says "Use constants" we are essentially saying the same thing from an "operational" perspective. Allow me to explain what I mean...

In either case - whether hard coded into the macro as you have done or hard coded in a constant as R2.3 is offering - the names are still hard coded within the VBA environment. In both cases, a change in personnel will require that someone goes into the VBA editor and replaces the name that is stored there. That requires someone with VBA knowledge to be involved with something that is essentially an HR activity.

If the names of the employees are stored in a worksheet and referenced by the code, then anyone that knows how to edit that worksheet (such as an HR employee) can replace Jo Doe with Jim Buck and the code will keep right on working with no developer involvement.

Yes, there will be some more coding upfront, but once done, the HR employee can deal with the personnel changes whenever and however often they need to without putting in a work order for a coding change.

message edited by DerbyDad03


Reply ↓  Report •

#11
January 10, 2019 at 01:08:29
Well, this debate is very educational.

1) I'll remove the On Error Resume Next and deal with this issue differently.

2) Having read R2.3's link (thanks for that), I have to agree that the constants route wouldn't really solve the issue for me. I work in a team where I'm seen as the 'Excel Guru'. I know, funny right? But essentially there's no one else in the team that would know to go into the VBA editor and change the names either within the code itself, or to change the constants, so this solution is a bit of a non-starter.

DerbyDad03, your suggestion about the personnel worksheet is a sensible one. The only thing is that the actual workbook is a dashboard for a manager that was originally created by someone else and was given to me to update. As part of the process of populating the "Data" sheet, another module in the workbook first hides everything except "Data", then hides "Data" and everything else apart from the "Workload" sheet and one other. I'll just need to decide whether to have the additional personnel sheet you suggest and add it to the visible list, add a discreet names list on one of the already visible sheets, or just leave it to the manager to contact me/another capable person to go in and change the code if/when necessary. I'm leaning towards putting the names list on the dashboard. Then I can have the column headers reference the list as well so that names only need to be changed once when necessary, but I'll ponder it.

3) With regard to this DerbyDad03 is exactly right. He helped me with the thread referenced at the top of my first post in this one so had that prior knowledge.

Thanks Both

message edited by ScottV


Reply ↓  Report •

#12
January 10, 2019 at 06:47:30
re: "1) I'll remove the On Error Resume Next and deal with this issue differently."

Hasn't it already been dealt with via the If Not IsDate instruction?

message edited by DerbyDad03


Reply ↓  Report •

#13
January 10, 2019 at 06:54:47
Yes. I didn't know if leaving it in the macro would cause a problem if a different error occurred for some reason.

Reply ↓  Report •

#14
January 10, 2019 at 07:16:23
Point still stands. There's no reason to go through the data once to find everything. Break it down into manageable chunks. Find out what code can be generalized and moved to its own function. If it feels too complex for you now, it'll only get worse when you have to come back to it.

How To Ask Questions The Smart Way


Reply ↓  Report •

#15
January 10, 2019 at 08:35:38
R2.3 - No argument as far your "manageable chunks" comments, however, I'm not sure what you mean by this:

"There's no reason to go through the data once to find everything."

Please explain, based on the task at hand. I'm always in learning mode, so I'd love to hear your thoughts on meeting ScottV's requirements.

How would you use VBA Functions to accomplish the tasks that were outlined in Response #9 of this thread?

The original input data and the requirements for the output can be found in the following thread. Other than changes to the columns, I believe that the data layout is consistent between that thread and what the code in this thread is running against. ScottV can verify that or update the data layout.

https://www.computing.net/answers/o...


message edited by DerbyDad03


Reply ↓  Report •

#16
January 11, 2019 at 01:18:20
To confirm, the original requirements were discussed in the thread DebyDad03 referenced in #15 above. As he mentions the code is consistent other than the data layout. The code posted in #1 above is what I'm now using, including the data layout.

Reply ↓  Report •

#17
January 11, 2019 at 06:05:00
ScottV,

I'm confused by your latest comments. You said: "The code posted in #1 above is what I'm now using, including the data layout."

The "data layout" in Post #1 in this thread does not appear to be the data that the code is using to build the output table discussed in your previous thread.

I don't recall seeing any input data that would produce the table you included in Post #1 of this thread.

Please explain.

How To Post Data or Code ---> Click Here Before Posting Data or VBA Code


Reply ↓  Report •

#18
January 11, 2019 at 06:20:47
Apologies. At the end of the previous thread I showed what I'd done with the manager who will use the workbook. They changed their requirements a bit. I was originally a bit stumped and was intending to come back to you with the relevant questions, but I worked it out myself in the end. I forgot that I hadn't actually discussed the change with you! :-)

The altered layout produced by the code in #1 above is the result of those changed requirements. Sorry for the confusion.


Reply ↓  Report •

#19
January 11, 2019 at 07:59:29
I'm still confused. Please keep in mind that we cannot see your workbook from where we are sitting, so you need to be very specific with the info that you provide. For the most part you have been very good at that and we thank you.

However, I don't recall seeing any input data, in either thread, that would produce the output table that you show in Post #1 of this thread. In the previous thread you posted 2 different tables of input data. The first table had "Ref" numbers like 1858, 1347, 985. The second table had "Ref" numbers like 1, 2, 3, etc.

The output table in Post #1 has what appear to be years: 1971, 1997, 2010, etc. I assume (which is a dangerous thing to do) that these are not actually years, but since this entire discussion is related to dates, it's confusing (at least to me) to see those types of reference numbers, especially since they were never used in any of the examples of input data that you've posted previously.

Since we typically have to set up input data tables in order to test solutions that will meet the stated requirements, consistency really helps.

As I often request in my responses: "Please provide a sample of your input data and the desired output based on that input."

The last 4 words of that request are key. If the example output doesn't have a clear relationship to the example input, we'll have a lot of trouble bridging the gap.

How To Post Data or Code ---> Click Here Before Posting Data or VBA Code


Reply ↓  Report •

#20
January 11, 2019 at 09:12:18
Again, sorry.

Firstly, the reference numbers from the first table are real. They aren't years but represent case IDs. The team started from 1 and increment with each new case. I understand the confusion when we were talking about dates, but they aren't.

Secondly, the reason I switched to 1, 2, 3 etc in the second table was because I was testing the code against a simplified data set and just dragged down in the ref column to create a quick set of numbers. I didn't appreciate it could cause confusion, but I understand now why it would. So again, to clarify, you can ignore the actual table structure from the second table.

This is the layout of the actual "Data" worksheet that the current code is working on. Below that I'll show you the layout of where the data is being copied to on the "Workload" sheet:

1  (Columns A:E contain formulas)   F   (Ignore columns G:P)       Q             R         S
2                                  Ref                        Date Assigned    Owner    Status
3                                 1858                        19/11/2018        Jo   Work in progress
4                                 1347                        05/07/2018       Laura Work in progress
5                                  985                        11/02/2017        Jo      Complete
etc

Then the Workload sheet outputs like this:

(Ignore columns A:R)  S     T     U     V     W     X     Y     Z   AA   AB       AC
1 
2
3
4                       30+         90+        180+        Other        Ref   Problematic
                                                                             Date Assigned
5                    Jo   Laura  Jo   Laura  Jo   Laura  Ref  Bracket   2035    15/12/20018
6                   1858   1871 1880  1898  1905         2001 180 Plus
7                   1907
8                   1995                                            

I've just used example reference numbers to show you how they lay out. Hopefully this makes more sense to you now.

Basically we get a list of each person's cases in each age bracket. The Other column is there to capture any cases that are in someone's name other than Jo or Laura.


Reply ↓  Report •

#21
January 11, 2019 at 13:01:37
Could you provide the latest version of the code also? This thread is a discussion related to modifications of the code that you posted in #1 and I'd like to see the most current version, hopefully a version that will create the output table in #20, based on the input in #20.

I'm just trying to tie up loose ends. Thanks!

How To Post Data or Code ---> Click Here Before Posting Data or VBA Code


Reply ↓  Report •

#22
January 12, 2019 at 21:50:38
Question: Are you only interested in "Laura" and "Jo"? Are there others on this sheet? Do you only care about them?

How To Ask Questions The Smart Way


Reply ↓  Report •

#23
January 14, 2019 at 00:31:50
DerbyDad03 I'll post the current code below. I've removed the On Error Resume Next, and took your advice re having the CI Manager's names in the workbook. The code now takes them from AE5:AE6 on the Workload sheet.

Razor2.3 The vast majority of the cases the code looks at are owned by Jo and Laura. However, there are a small few (one or two I think) that are owned by someone else. I have a section in the code to pick these up:

If ciMgr <> Sheets("Workload").Range("AE5") And ciMgr <> Sheets("Workload").Range("AE6") Then
          nxtDstRw = .Cells(Rows.Count, 25).End(xlUp).Row + 1
         .Cells(nxtDstRw, "Y") = Sheets("Data").Cells(myDates, "F")
         .Cells(nxtDstRw, "Z") = new_bracket
        End If

So here's the code in full that is currently in use:

Sub DateBrackets()

'Determine last row with data in case date bracket moves on "Workload" sheet _
note using Find because some columns are longer than others in the range _
as well as empty columns
clrBrkData = Cells.Find(What:="*", After:=Sheets("Workload").Range("A1"), _
            LookAt:=xlPart, LookIn:=xlFormulas, _
            SearchOrder:=xlByRows, SearchDirection:=xlPrevious, _
            MatchCase:=False).Row

'Clear date bracket cases from "Workload" sheet, Leave Header Row
  Sheets("Workload").Range("S6:Z" & clrBrkData).ClearContents
  
'Clear incorrectly formatted dates info from columns AB:AC, Leave Header Row
  Sheets("Workload").Range("AB5:AC" & clrBrkData).ClearContents

Application.ScreenUpdating = False

'Determine Last Row with data on "Data" sheet
  lastSrcRw = Sheets("Data").Cells(Rows.Count, "F").End(xlUp).Row

'Loop through Column S, Status must be "Work in progress"
    For myDates = 2 To lastSrcRw
     If Sheets("Data").Cells(myDates, "S") = "Work in progress" Then
     
'Validate date in Column Q on "Data" sheet
       If Not IsDate(Sheets("Data").Cells(myDates, "Q")) Then
       With Sheets("Workload")
        nxtBadDt = .Cells(Rows.Count, 28).End(xlUp).Row + 1
        .Cells(nxtBadDt, "AB") = Sheets("Data").Cells(myDates, "F")
        .Cells(nxtBadDt, "AC") = Sheets("Data").Cells(myDates, "Q")
       End With
       Else
 
 'Calculate Case Age from Column Q
      caseAge = Date - Sheets("Data").Cells(myDates, "Q")
      
      End If
           
'Assign CI Mgr name from names entered on "Workload" sheet in AE5:AE6
      ciMgr = Sheets("Data").Cells(myDates, "R")
   
'Set New Bracket based on Case Age
      If caseAge < 181 Then
        If 180 - caseAge < 28 Then
           new_bracket = "180 Plus"
        ElseIf 91 - caseAge < 14 Then
           new_bracket = "91 to 180"
        ElseIf 30 - caseAge < 7 Then
           new_bracket = "31 to 90"
        End If
      End If

     
 'If new Bracket required, copy case references only from column "F" _
 on "Data" sheet to the case bracket columns on the "Workload" sheet, _
 as well as the references and date brackets for any cases not belonging _
 to the key team members
 
 'Also note that the key team members names are found in the "Workload" sheet _
 in cells AE5 and AE6. If the team expands then the code below will need to be _
 amended to extend the ranges to take the names from.
       If new_bracket <> "" Then
      With Sheets("Workload")
        If new_bracket = "31 to 90" Then
          If ciMgr = Sheets("Workload").Range("AE5") Then
          nxtDstRw = .Cells(Rows.Count, 19).End(xlUp).Row + 1
         .Cells(nxtDstRw, "S") = Sheets("Data").Cells(myDates, "F")
          ElseIf ciMgr = Sheets("Workload").Range("AE6") Then
          nxtDstRw = .Cells(Rows.Count, 20).End(xlUp).Row + 1
         .Cells(nxtDstRw, "T") = Sheets("Data").Cells(myDates, "F")
          End If
        End If
        
        If new_bracket = "91 to 180" Then
          If ciMgr = Sheets("Workload").Range("AE5") Then
          nxtDstRw = .Cells(Rows.Count, 21).End(xlUp).Row + 1
         .Cells(nxtDstRw, "U") = Sheets("Data").Cells(myDates, "F")
          ElseIf ciMgr = Sheets("Workload").Range("AE6") Then
          nxtDstRw = .Cells(Rows.Count, 22).End(xlUp).Row + 1
         .Cells(nxtDstRw, "V") = Sheets("Data").Cells(myDates, "F")
          End If
        End If
        
        If new_bracket = "180 Plus" Then
          If ciMgr = Sheets("Workload").Range("AE5") Then
          nxtDstRw = .Cells(Rows.Count, 23).End(xlUp).Row + 1
         .Cells(nxtDstRw, "W") = Sheets("Data").Cells(myDates, "F")
          ElseIf ciMgr = Sheets("Workload").Range("AE6") Then
          nxtDstRw = .Cells(Rows.Count, 24).End(xlUp).Row + 1
         .Cells(nxtDstRw, "X") = Sheets("Data").Cells(myDates, "F")
          End If
        End If
          
        If ciMgr <> Sheets("Workload").Range("AE5") And ciMgr <> Sheets("Workload").Range("AE6") Then
          nxtDstRw = .Cells(Rows.Count, 25).End(xlUp).Row + 1
         .Cells(nxtDstRw, "Y") = Sheets("Data").Cells(myDates, "F")
         .Cells(nxtDstRw, "Z") = new_bracket
        End If
    End With
        
           
'Clear New Bracket flag
      new_bracket = ""
      End If
    End If
      
    Next

'Alert the user that there are cases with incorrectly formatted dates that need action
If Sheets("Workload").Range("AB5") <> "" Then
    MsgBox "The SOFIT database contains incorrect dates in the Date Assigned column." _
    & vbNewLine & vbNewLine & "Please review which submissions need correction and take appropriate" _
    & "corrective action.", vbOKOnly + vbExclamation, "ATTENTION!"
End If
    
End Sub



Reply ↓  Report •

#24
January 14, 2019 at 06:30:22
ScottV: I hope you don't mind, but I took the liberty to make a minor edit to your code.

Some of your code comments were so "wide" that they forced the text section of your post to extend beyond the right border of the message pane. This then forces the reader to scroll left and right in order to read the text portion your post.

This issue is caused by the fact that the pre tags take precedence over all else. The width of the entire post will be based on the longest line within the pre tags, so the "extra wide" comments impact the text portion also.

What I did was sort of re-format your comments so that no single line would force the text beyond the borders of the message pane. A couple of line continuation characters to split the comments into shorter lines was all it took.

If that is a problem, you can simply edit your post and put it back the way it was.

Thanks for understanding.


message edited by DerbyDad03


Reply ↓  Report •

#25
January 14, 2019 at 07:34:05
Are you calling my comments fat????

:-) Haha, no problem.


Reply ↓  Report •

#26
January 15, 2019 at 22:04:52
I spend more time in PowerShell in a day than I do in Excel in a week, and y'all want want me to break this down into bite size VBA chunks? 'Kay.

DerbyDad03: 1 - Search a specific column for a specific string ("Work in Progress")
Well, Excel has Range.Find and Range.FindNext, so that should be simple enough. We'll need to keep track of the first cell found, so we don't end up in an endless loop. We'll stick our rows together with Application.Union. There's one more caveat we need to worry about, namely we don't find what we're looking for.

Sub SomethingMeaningfulImSure()
  'Normally I'd just use named ranges
  Dim status As Range: Set status = [S:S]
  Dim owner As Range: Set owner = [R:R]
  Dim dAssigned As Range: Set dAssigned = [Q:Q]
  Dim ref As Range: Set ref = [F:F]
  
  Dim c As Range, wip As Range, start As String
  Set c = status.Find("Work in progress", , xlValues)
  If c Is Nothing Then _
    Exit Sub 'Nothing to do!
  Set wip = c: start = c.Address
  Do
    Set wip = Union(wip, c)
    Set c = status.FindNext(c)
  Loop Until c.Address = start
End Sub

Now, ultimately we're interested in "Laura" and "Jo." No one's using their words to explain what happens to anyone else found, so I'll just ignore that situation! Since I'm talking about breaking things down, let's just look for jobs involving "Laura." Excel has Range.Find and Range.FindNext, so that should be simple enough. Wait a second, this seems familiar. By replacing the constants with parameters, we could use the same code block as a Function. I'm also skipping half a step here, and returning entire rows.

Sub SomethingMeaningfulImSure()
    'Normally I'd just do this as named ranges
    Dim status As Range: Set status = [S:S]
    Dim owner As Range: Set owner = [R:R]
    Dim dAssigned As Range: Set dAssigned = [Q:Q]
    Dim ref As Range: Set ref = [F:F]
    
    Dim wip As Range, laura As Range, jo As Range
    Set wip = FindRows(status, "Work in progress")
    Set laura = FindRows(owner, "Laura")
    Set jo = FindRows(owner, "Jo")
End Sub

Function FindRows(searchRange As Range, searchFor As String) As Range
  Dim c As Range, start As String
  Set c = searchRange.Find(searchFor, , xlValues)
  Set FindRows = c
  If c Is Nothing Then _
    Exit Function 'Nothing to do!
  start = c.Address
  Do
    Set FindRows = Union(FindRows, c.EntireRow)
    Set c = searchRange.FindNext(c)
  Loop Until c.Address = start
End Function

Now we have three lists. Everything marked "Work in progress," everything involving "Laura", and everything involving "Jo." How do we compare them? By using Application.Intersect. This is why we needed the entire rows. Disclaimer here, but this isn't how I'd normally go about this. It's just the most direct way to get from where we were to where we need to be.

Dim wip As Range, laura As Range, jo As Range
Set wip = FindRows(status, "Work in progress")
Set laura = FindRows(owner, "Laura")
Set laura = Intersect(laura, wip)
Set jo = FindRows(owner, "Jo")
Set jo = Intersect(jo, wip)

By my count, that's two thirds of of the problem done. Only thing left to do is determine if a record is within a bracket. Well, what what does it mean to be in a bracket? The age has to be greater than or equal to the starting age, and less then the ending age, assuming there is an end. Pretty straight forward. We're going to wrap this up in another Function. Not because we're calling it from multiple places like FindRows, but because it provides seperation from the actual test and the main code. What does that look like in VBA?

Function InBracket(ByVal check As Date, ByVal start As Integer, _
                        Optional ByVal ending As Integer = 0) As Boolean
  Dim age As Integer: age = Now - check
  InBracket = age >= start And (age < ending Or ending = 0)
End Function

You can ignore the ByVals; they're working around a VBA parameter passing issue. Now that we have all of the pieces, save the output. I'm not sure what this "other" is in the example output, and no one's using their words to explain it, so it must not be important. Ignored! Also no one's talking about valid but future dates, so that one job Jo had assigned to her in the year 2020 is safe for another year. Must not be important. Ignored! Back on track, the output. I see no reason not to use heavily nested For loops, so I will. I'm also cheating by using Scripting.Dictionary, but I don't care.

'Not shown: Cleaning up the output area
Dim outStart As Range, out As Range, person, recDate As Range
Set outStart = [OutputPage!S6]
Dim brackets As Object
Set brackets = CreateObject("Scripting.Dictionary")
brackets(30) = 90: brackets(90) = 180: brackets(180) = 0
'Order matters when nesting For loops
For Each bracket In brackets.Keys
  For Each person In Array(jo, laura)
    Set out = outStart: Set outStart = outStart.Offset(0, 1)
    For Each rec In person.EntireRow
      Set recDate = Intersect(rec, dAssigned)
      If IsDate(recDate) Then
        If InBracket(recDate, bracket, brackets(bracket)) Then
          out.Formula = recDate
          Set out = out.Offset(1, 0)
        End If
      End If
    Next 'rec
  Next 'person
Next 'bracket

Three nested For loops and two nested If statements. That was the largest chunk of code yet. Anyways, the non-date dates. I see no reason not to do yet another pass for this.

Dim errOut As Range: Set errOut = [OutputPage!AB5:AC5]
For Each rec In wip.EntireRow
  Set recDate = Intersect(rec, dAssigned)
  If Not IsDate(recDate) Then
    errOut = Array(Intersect(rec, ref), recDate)
    Set errOut = errOut.Offset(1, 0)
  End If
Next 'rec

Piece it all together, and the entire thing looks like:

Sub SomethingMeaningfulImSure()
  'Normally I'd just use named ranges
  Dim status As Range: Set status = [S:S]
  Dim owner As Range: Set owner = [R:R]
  Dim dAssigned As Range: Set dAssigned = [Q:Q]
  Dim ref As Range: Set ref = [F:F]
  
  Dim wip As Range, laura As Range, jo As Range
  Set wip = FindRows(status, "Work in progress")
  Set laura = FindRows(Intersect(owner, wip), "Laura")
  Set jo = FindRows(Intersect(owner, wip), "Jo")

  'Not shown: Cleaning up the output area
  Dim outStart As Range, out As Range, person, recDate As Range
  Set outStart = [OutputPage!S6]
  Dim brackets As Object
  Set brackets = CreateObject("Scripting.Dictionary")
  brackets(30) = 90: brackets(90) = 180: brackets(180) = 0
  For Each bracket In brackets.Keys
    For Each person In Array(jo, laura)
      Set out = outStart: Set outStart = outStart.Offset(0, 1)
      For Each rec In person.EntireRow
        Set recDate = Intersect(rec, dAssigned)
        If IsDate(recDate) Then
          If InBracket(recDate, bracket, brackets(bracket)) Then
            out.Formula = recDate
            Set out = out.Offset(1, 0)
          End If
        End If
      Next 'rec
    Next 'person
  Next 'bracket
  Dim errOut As Range: Set errOut = [OutputPage!AB5:AC5]
  For Each rec In wip.EntireRow
    Set recDate = Intersect(rec, dAssigned)
    If Not IsDate(recDate) Then
      errOut = Array(Intersect(rec, ref), recDate)
      Set errOut = errOut.Offset(1, 0)
    End If
  Next 'rec
End Sub

Function FindRows(searchRange As Range, searchFor As String) As Range
  Dim c As Range, start As String
  Set c = searchRange.Find(searchFor, , xlValues)
  Set FindRows = c
  If c Is Nothing Then _
    Exit Function 'Nothing to do!
  start = c.Address
  Do
    Set FindRows = Union(FindRows, c.EntireRow)
    Set c = searchRange.FindNext(c)
  Loop Until c.Address = start
End Function

Function InBracket(ByVal check As Date, ByVal start As Integer, _
                   Optional ByVal ending As Integer = 0) As Boolean
  Dim age As Integer: age = Now - check
  InBracket = age >= start And (age < ending Or ending = 0)
End Function

How To Ask Questions The Smart Way

message edited by Razor2.3


Reply ↓  Report •

Ask Question