0

I am working on a macro that provides a visual representation of the 4 different production phases of a product we produce. The spreadsheet is set up to appear as a linear calendar, showing all 365 days of the year and grouping them into calendar weeks. Each production phase has a different color associated with it. The four buildphases are: 1) Assembly: 1 day (yellow) 2) Initial analysis: 1 day (purple) 3) In-depth analysis: 7 days (green) 4) Shipping: 1 day (red)

No work is performed in the facility on weekends or holidays. Saturdays and sundays are represented by cells colored black and holidays are represented by cells colored orange and having a criss cross pattern. The macro is designed to skip these holidays using the following if then statement:

Dim i As Integer

i = Analysis days

for i = 1 to 7

ActiveCell.Select

If Selection.Interior.Pattern = x1CrissCross And Selection.Interior.Color = orange Then

    ActiveCell.Offset(0, 1).Select

Else: ActiveCell.Select

End If

If Selection.Interior.Color = black Then

    ActiveCell.Offset(0, 2).Select

Else: ActiveCell.Select

End If

The If Then statement that tells the macro to skip Saturday and Sunday works every time. But the statement that tells the macro to skip holidays only works part of the time, and if the holidays last longer than a few days (Christmas vacation lasts nine days) the macro inserts production work days interspersed throughout the holiday. I've found that copying and pasting the above statements directly beneath one another several times seems to provide a quick fix of the issue. But I am sure there has to be a more efficient way to do this. Does anyone know of a way I can fix the issue without having to copy and paste the same lines of code mulitple times?

Thanks in advance!

Scott Craner
  • 148,073
  • 10
  • 49
  • 81
Alec Terry
  • 37
  • 1
  • 6
  • This is pretty ambitious ^_^; – findwindow Apr 20 '16 at 19:02
  • It's tedious to color it by hand but that may be far less tedious than the logic required for a macro? – findwindow Apr 20 '16 at 19:09
  • Ahh, I ***highly*** suggest you read through [How to avoid `.Select`](http://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba-macros), it will save you many headaches, and will help learn how VBA works/thinks. – BruceWayne Apr 20 '16 at 19:16
  • Did you use `Option Explicit` at the top of your module? I think `x1CrissCross` should be `xlCrissCross` or `xlPatternCrissCross` – OldUgly Apr 20 '16 at 22:41

1 Answers1

0

One of your issues is to do with the logic of your loop, because you always "process" the next cell without testing it (for the holidays). Also as mentioned above the x1crisscross should be xlcrisscross, and I dont think orange is a "defined" color so you may need to get the value for that and test against it (see myorange below).

A loop like this would avoid that:

' I think you need to need to define what color orange is - as its not one of the "standard" named ones
myOrange = 4626167

For i = 1 To 7
    Debug.Print Selection.Interior.Color
    Debug.Print Selection.Interior.Pattern

    If (Selection.Interior.Color = black Or (Selection.Interior.Color = myOrange And Selection.Interior.Pattern = xlCrissCross))  Then
        ' skip this one so decrement counter
        i = i - 1
    Else
        ' Do the "stuff" for a valid cell here - for testing I just put the counter in the cell
        ActiveCell.Value = i
    End If
    ' move to next cell
    ActiveCell.Offset(0, 1).Select
Next i

Personally I never like fiddling with the "counter" like this, it just seems messy to decrement it.

The other way to get the same affect is to have a loop within the for/next loop that keeps checking until it finds a valid cell. 

myOrange = 4626167

For i = 1 To 7
    found1 = False
    While Not found1
        If Not ( Selection.Interior.Color = black Or (Selection.Interior.Color = myOrange And Selection.Interior.Pattern = xlCrissCross))  Then
            found1 = True
        Else
            ' move to next cell as this one isnt available
            ActiveCell.Offset(0, 1).Select
        End If
    Wend
    ' Do the "stuff" for a valid cell here
     ActiveCell.Value = i
    ' move to next cell
    ActiveCell.Offset(0, 1).Select

Next i

I also added the test for weekends (black) and holidays into the same If statement (notice the brackets to make sure the or and and are processed correctly). I always tend to put them just to make it clearer.

Of course you could do all this using a counter and "range" etc... and not selecting the cell but the end result will be the same.

Both these methods "could" loop forever if there are no more valid cells so it might be worth testing for some maximum column count and breaking out if you reach that.

NanoTera
  • 61
  • 5