A complete beginner's solution to nested If-Then constructs

[stefansi]

Hi all,

I'm an absolute beginner in vba (started two weeks ago). In short, can you tell me why the code always skips the part in red font and goes straight from the "Then" statement in blue to the "Else" statement in blue, irrespective of whether the statement If Sheet.Name = UCase(szToday) is True or False? The code is supposed to copy the sheet Accounts and rename it with today's date ("ddmmm"), but before that happens the code is supposed to go through all the sheets in the workbook and check if any of the sheets' name is today's date ("ddmmm" - the sheet Accounts has therefore aldready been copied and renamed). If the statement Sheet.Name = UCase(szToday) is true, then the MsgBox should appear and ask the user if he wants to make another copy or not (without renaming it though). The code below this one does the same thing and works fine, in case you want to see how it actually works.


Sub CopySht()

Dim szToday As String
szToday = Format(Date, "ddmmm")

For Each Sheet In Worksheets
If Sheet.Name = UCase(szToday) Then

If MsgBox("Copied already! Do you want another?", vbYesNo + vbQuestion) = vbYes Then
Sheets("Accounts").Select
Sheets("Accounts").Copy After:=Sheets(Sheets.Count)
Exit Sub

Else
Worksheets(1).Select
Exit Sub

End If

Else

Sheets("Accounts").Select
Sheets("Accounts").Copy After:=Sheets(Sheets.Count)
ActiveSheet.Name = UCase(szToday)

Exit Sub

End If
Next Sheet

End Sub



The following code performs the action I was looking to do with the code above (run it twice and you see what I mean). However, the book I'm reading (John Walkenbach's Excel Power Programming with VBA) states that I should refrain from using a GoTo statement unless that's the only way to perform an action.

Sub CopySht()

Dim szToday As String
szToday = Format(Date, "ddmmm")

For Each Sheet In Worksheets
If Sheet.Name = UCase(szToday) Then GoTo AnsBox
Next Sheet

Sheets("Accounts").Select
Sheets("Accounts").Copy After:=Sheets(Sheets.Count)
ActiveSheet.Name = UCase(szToday)

Exit Sub

AnsBox:
If MsgBox("Copied already! Do you want another?", vbYesNo + vbQuestion) = vbYes Then

Sheets("Accounts").Select
Sheets("Accounts").Copy After:=Sheets(Sheets.Count)
Exit Sub

Else
Worksheets(1).Select
Exit Sub

End If

End Sub

Many thanks in advance. One more thing, in case it matters. I'm not a native speaker of English, so take that into account if some of the things I write look awkward.

Thanks,
Stefan

 

[misterpinkey]

Your code was setup to loop through each sheet name; for every sheet where the name did not equal UCase(szToday), then it would perform the else loop.

Try:

Sub CopySht()
Dim szToday As String
szToday = Format(Date, "ddmmm")
Dim blnSheetAlreadyExists As Boolean

For Each Sheet In Worksheets
    If Sheet.Name = UCase(szToday) Then blnAlreadyExists = True
Next Sheet
    
If blnSheetAlreadyExists = True Then
    If MsgBox("Copied already! Do you want another?", vbYesNo + vbQuestion) = vbYes Then
        Sheets("Accounts").Select
        Sheets("Accounts").Copy After:=Sheets(Sheets.Count)
        Exit Sub
    Else
        Worksheets(1).Select
        Exit Sub
    End If
Else
    Sheets("Accounts").Select
    Sheets("Accounts").Copy After:=Sheets(Sheets.Count)
    ActiveSheet.Name = UCase(szToday)
    Exit Sub
End If
End Sub

 

[stefansi]

Thank you misterpinkey! This solves my problem

Stefan

 

[misterpinkey]

Sorry, small typo fixed below (unable to edit my original since it's > 10 minutes). I added some comments to the code too.

Sub CopySht()
Dim szToday As String
Dim blnSheetAlreadyExists As Boolean 'Boolean is just a true/false variable

szToday = Format(Date, "ddmmm")

'Loop through each sheet & check if one exists already matching UCase(szToday). Store the result to a boolean variable (true if exists, false if not; boolean is false by default)
For Each Sheet In Worksheets
    If Sheet.Name = UCase(szToday) Then blnSheetAlreadyExists = True
Next Sheet
    
'If a sheet with the name already exists, then this portion of the code will execute:
If blnSheetAlreadyExists = True Then
    If MsgBox("Copied already! Do you want another?", vbYesNo + vbQuestion) = vbYes Then
        Sheets("Accounts").Select
        Sheets("Accounts").Copy After:=Sheets(Sheets.Count)
        Exit Sub
    Else
        Worksheets(1).Select
        Exit Sub
    End If
'If a sheet with the name does not exist, then this portion of the code will execute:
Else
    Sheets("Accounts").Select
    Sheets("Accounts").Copy After:=Sheets(Sheets.Count)
    ActiveSheet.Name = UCase(szToday)
    Exit Sub
End If
End Sub