1

For some reason my code is not working. I am getting "Error 91". I ran it fine with a "For each loop but this is not working for me.

It is telling me to debug at 

strPrint = MsgBox(prompt:="Print " & shtCurrent & "?", Buttons:=vbYesNo + vbExclamation)

but I cannot find anything wrong with it. 

Public Sub PrintWorksheets2()

    'declare variables and assign address
    Dim strPrint As String, intCount As Integer, wkbHours As Workbook, shtCurrent As Worksheet
    Dim intSum As Integer, shtCount As Integer
    Set wkbHours = Application.Workbooks("auco6215_HW10_Ex9.xlsm")

    shtCount = wkbHours.Sheets.Count
    intSum = 0

    'ask user if he or she wants to print the worksheet
    For intCount = 1 To shtCount
        'shtCurrent = wkbHours
        intSum = intSum + intCount
        strPrint = MsgBox(prompt:="Print " & shtCurrent & "?", Buttons:=vbYesNo + vbExclamation)
        If strPrint = vbYes Then        'if user wants to print
            shtCurrent.PrintPreview
        End If
    Next intCount

End Sub
Improve this question
11
  • Try declaring strPrint as variant instead of string. Commented Nov 4, 2014 at 1:51
  • Okay I will. Although, I declared it as a string in my "For Each" loop? Commented Nov 4, 2014 at 1:55
  • 2
    @bodjo, good catch. Even better would be VbMsgBoxResult. Commented Nov 4, 2014 at 1:56
  • @DougGlancy as the variable name and then variant as the data type? Commented Nov 4, 2014 at 2:00
  • 1
    You never assign (Set) shtCurrent = to a worksheet. That will --> error 91. Also, you probably want shtCurrent.Name in your MsgBox Commented Nov 4, 2014 at 2:01

2 Answers 2

Reset to default
3

shtCurrent is a Worksheet object. Try concatenating its Name property (which is a String) into your msgbox prompt:

strPrint = MsgBox(prompt:="Print " & shtCurrent.Name & "?", Buttons:=vbYesNo + vbExclamation)

Now strPrint is declared as a String:

Dim strPrint As String

And MsgBox will return a vbMsgBoxResult enum value - you're making VBA do lots of useless implicit type conversions here, why not declare it as the type it's returned as?

Dim strPrint As vbMsgBoxResult

And now you see why prefixing str to variable names (aka Hungarian notation) is bad.

That only addresses part of the issue. When you had a For Each loop, each iteration assigned a value to an object variable that represented the current iteration's worksheet. In this code with a For loop, you have a shtCurrent worksheet object, but it's not assigned; you're incrementing a counter, but that's all you're doing. As was hinted in the comments, you also need to Set the reference for the shtCurrent object inside the loop, before you use it - otherwise your code will blow up with an "object variable not set" error.

Your For loop iterates worksheet indices, so you can assign shtCurrent to an item in the Worksheets collection:

Set shtCurrent = Worksheets(intCount)

Do that before anything else inside the body of the For loop, and you should be good to go.

Now that's all nice, but when you're iterating objects, it's often better to use a For Each loop instead of a For loop. For loops are good for iterating arrays; For Each loops are good for iterating collections - I'd use a For Each loop here.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

If we declare as Dim strPrint As String and if we check msgbox result like this If strPrint = vbYes Then. It is still inappropriate way and as you said it will make VBA to do implicit type conversion for this statement too?
@PareshJadhav vbYes is a member of the VbMsgBoxResult enum type, and has an underlying Integer value of 6 - in the case you describe, when the user clicks the "Yes" button, the result of the MsgBox call stored in strPrint will be converted to "6" so as to store the result in the allocated String variable, and VB will implicitly convert vbYes to its "6" string representation before it compares it to strPrint in the if strPrint = vbYes expression... so yes.
Thanks RetailCoder for explanation. One last doubt. If we declare variable as VbMsgBoxResult then implicit conversion is required or not?
By declaring strPrint As VbMsgBoxResult, you're storing the result of the MsgBox call in a VbMsgBoxResult variable, that will have an underlying value of 6 when the user clicks the "Yes" button; when you compare if to vbYes in the if strPrint = vbYes expression, VBA doesn't need to convert anything to perform the comparison, it just evalues If 6 = 6 Then and no string is ever involved; no string is needed here.
0

Please try this.

'declare variables and assign address
Dim strPrint As String, intCount As Integer, wkbHours As Workbook, shtCurrent As Worksheet
Dim intSum As Integer, shtCount As Integer
Dim oResult
Set wkbHours = Application.Workbooks("auco6215_HW10_Ex9.xlsm")

shtCount = wkbHours.Sheets.Count
intSum = 0

'ask user if he or she wants to print the worksheet
For intCount = 1 To shtCount
    'shtCurrent = wkbHours
    intSum = intSum + intCount
    oResult = MsgBox(prompt:="Print " & shtCurrent & "?", Buttons:=vbYesNo + vbExclamation)
    If oResult = vbYes Then        'if user wants to print
        shtCurrent.PrintPreview
    End If
Next intCount
Improve this answer

3 Comments

I got error 91 still "Object variable or with block variable not set". It doesn't like the line "vbResult = MsgBox(prompt:="Print " & shtCurrent & "?", Buttons:=vbYesNo + vbExclamation"
I haven't tested the code. But i changed vbResult to oResult. Looks like shtCurrent is the one causing the error as mentioned by @Ron.
I will give that a go. What should the data type be, variant?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.