Reputation: 11
I want to format a shape (Oval 1) using cell value "A1" I want to format the next shape (Oval 2) using cell value "A2" and so on.. I can get this done with repeated "If", "Else if" statements but I'm looking for a way to achieve this with a "for" loop or some other method to reduce the if/else statements and make the code simpler yet still effective.
Sub format_connector()
'
Application.ScreenUpdating = False
On Error Resume Next
ActiveSheet.Unprotect
ActiveSheet.Shapes.Range(Array("Oval 1")).Select
If Range("D5") = "GREEN" Then
Call green
ElseIf Range("D5") = "YELLOW" Then
Call yellow
ElseIf Range("D5") = "BLACK" Then
Call black
ElseIf Range("D5") = "BLACK/WHITE" Then
Call black_white
ElseIf Range("D5") = "RED" Then
Call red
ElseIf Range("D5") = "RED/WHITE" Then
Call red_white
ElseIf Range("D5") = "ORANGE" Then
Call orange
ElseIf Range("D5") = "ORANGE/WHITE" Then
Call orange_white
ElseIf Range("D5") = "BLUE" Then
Call blue
ElseIf Range("D5") = "BLUE/WHITE" Then
Call blue_white
ElseIf Range("D5") = "BROWN" Then
Call brown
ElseIf Range("D5") = "BROWN/WHITE" Then
Call brown_white
ElseIf Range("D5") = "VIOLET" Then
Call violet
ElseIf Range("D5") = "GRAY" Then
Call gray
ElseIf Range("D5") = "WHITE" Then
Call white
ElseIf Range("D5") = "WHITE/BLACK" Then
Call white_black
ElseIf Range("D5") = "WHITE/BLUE" Then
Call white_blue
ElseIf Range("D5") = "WHITE/BROWN" Then
Call white_brown
ElseIf Range("D5") = "408-4001-882" Then
Call cavity_plug
ElseIf Range("D5") = "408-4001-445" Then
Call cavity_plug
ElseIf Range("D5") = "408-4002-073" Then
Call cavity_plug
ElseIf Range("D5") = "408-4001-935" Then
Call cavity_plug
ElseIf Range("D5") = "BLANK" Then
Call blank
End If
Upvotes: 1
Views: 55
Reputation: 14383
How about this solution?
Sub format_connector()
' 008 26 Apr 2020
Dim Ws As Worksheet
Dim SubName As String
Dim R As Long
Dim i As Integer
Set Ws = ActiveSheet ' Be safe! Call the sheet by name.
On Error Resume Next
Ws.Unprotect
On Error GoTo 0
Application.ScreenUpdating = False
R = 5 ' start the loop at D5
Do
SubName = Replace(Ws.Cells(R, "D").Value, "/", "_")
If SubName = "" Then Exit Do
R = R + 1
i = i + 1
Ws.Shapes("Oval " & i).Select
If Val(SubName) Then SubName = "cavity_plug"
Application.Run SubName
Loop
Application.ScreenUpdating = True
End Sub
The above code was modified to select Ovals 1, 2 etc. for as many as there are parameters specified in column D, starting from D5.
Upvotes: 0
Reputation: 11
Sub FormatConnector()
Application.ScreenUpdating = False On Error Resume Next Call underscore 'converts to underscore between colors
Dim color As String
ActiveSheet.Shapes.Range(Array("Oval 1")).Select color = Range("D5").Value Call CAVITY_PLUG End If Application.Run color
ActiveSheet.Shapes.Range(Array("Oval 2")).Select
color = Range("D6").Value Application.Run color
ActiveSheet.Shapes.Range(Array("Oval 3")).Select
color = Range("D7").Value Application.Run color
Upvotes: 0
Reputation: 71187
Since every condition uses the same left operand in a comparison, the If...Else If...End If
block could be expressed with a Select...Case...End Select
block, and that would already reduce some of the repetition.
Private Function GetMacroName(ByVal source As Range) As String
Select Case Range("D5")
Case "YELLOW":
GetMacroName = "yellow"
Case "BLACK":
GetMacroName = "black"
Case "BLACK/WHITE"
GetMacroName = "black_white"
'...
Case Else
GetMacroName = "blank"
End Select
End Function
And then you can use Application.Run
to invoke the parameterless procedure:
Application.Run GetMacroName(Range("D5"))
You can use a loop to run this instruction for different ranges:
Dim sheet As Worksheet
Set sheet = ActiveSheet '<~ sure of that?
Dim i As Long
For i = 1 To N '<~ N=number of iterations; presumably the number of oval shapes
Dim oval As Shape
On Error Resume Next '<~ manually handle non-existing shape #i
Set oval = sheet.Shapes("Oval " & i)
On Error GoTo 0
If Not oval Is Nothing Then
Application.Run GetMacroName(sheet.Range("D" & 5 + i - 1)), oval
End If
Set oval = Nothing
Next
Alternatively we could iterate the sheet's Shapes
collection:
Dim sheet As Worksheet
Set sheet = ActiveSheet '<~ sure of that?
Dim oval As Shape, i As Long
For Each oval In sheet.Shapes
i = i + 1
If Left(oval.Name, 4) = "Oval" Then
Application.Run GetMacroName(sheet.Range("D" & 5 + i)), oval
End If
Next
Note that in both cases, the macro receives the Shape
object it works with. By passing parameters, you make your code less reliant on global state, and easier to follow: you shouldn't have to work out who's the caller 3 frames up the call stack that called .Select
on some shape, to know what you're working with! Passing parameters makes things much simpler to debug later.
Public Sub Yellow(ByVal sh As Shape)
sh.ForeColor = vbYellow
End Sub
Note that if that's all that's going on here, you should only have one macro instead:
Public Sub FormatOvalShape(ByVal oval As Shape, ByVal color As Long)
oval.ForeColor = color
'...
End Sub
..and let the caller parameterize the call - you'll reduce code duplication even further. Actually, it would remove the need to map macro names; instead we map color codes:
Dim sheet As Worksheet
Set sheet = ActiveSheet '<~ sure of that?
Dim oval As Shape, i As Long
For Each oval In sheet.Shapes
If Left(oval.Name, 4) = "Oval" Then
FormatOvalShape oval, GetColorCode(sheet.Range("D" & 5 + i))
End If
i = i + 1
Next
Upvotes: 3