3

I want to handle the click on an CommandButton from within a class using the following class code

Option Explicit

Private m_First                     As MSForms.CommandButton
Private WithEvents evFirst          As MSForms.CommandButton

Property Get First() As MSForms.CommandButton
    Set First = m_First
End Property
Property Let First(ByRef o As MSForms.CommandButton)
    Set m_First = o
    Set evFirst = o
End Property

Private Sub evFirst_Click()
    MsgBox "It Worked!"
End Sub

In addition to it not working, am wondering why the reference for the Button in the form is different from that in the class, ie:

Sub Tester()
    Dim f As New UserForm1
    Dim o As New cButtonClass
    o.First = f.CommandButton1

    Dim k1 As LongLong: k1 = ObjPtr(o.First)
    Dim k2 As LongLong: k2 = ObjPtr(f.CommandButton1)
    Debug.Assert k1 = k2 'NOPE!
End Sub

Why doesn't this work? What is the fix?

Zoe
  • 27,060
  • 21
  • 118
  • 148
Berryl
  • 12,471
  • 22
  • 98
  • 182
  • 2
    There are 2 problems I see. First, in your class change the `Property Let` to `Property Set`. Second, in the form code you need to say `Set o.First`. There may be other issues, too. – Brian M Stafford Oct 10 '18 at 19:14
  • 1
    Also, the `evFirst` object is identical to the `m_First` object. The `WithEvents` keyword simply allows you to write custom code for object you declared `WithEvents` for. Essentially, you don't need that `m_First` variable.... – Mistella Oct 10 '18 at 19:29
  • @BrianMStafford. No, you can use Let like a Set. You can make the case that you shouldn't from a coding style, but you can.(https://stackoverflow.com/questions/5042379/in-vb6-what-is-the-difference-between-property-set-and-property-let) – Berryl Oct 10 '18 at 20:15
  • @Mistella. Yes, you are right. I do want that m_First for other reasons if I get this working, but it isn't doing anything in this simplified code I made to illustrate the problem – Berryl Oct 10 '18 at 20:17
  • 2
    It's not a coding style thing. It's about meeting expectations. Also, if your class has a *default member*, let-coercion will literally change how the member works, with identical code - that's objectively wrong, not a style thing at all. One is correct, the other is begging for future bugs to happen. – Mathieu Guindon Oct 10 '18 at 20:24
  • Side note, the `RHS` argument of a `Property Let` or `Property Set` member is *always* passed `ByVal`, regardless of what the signature says: that explicit `ByRef` is misleading. – Mathieu Guindon Oct 10 '18 at 20:40
  • @MathieuGuindon. I didn't know that; does this explain the different ObjPtrs? – Berryl Oct 10 '18 at 20:46
  • I don't think so. It's not clear what `UserForm1.CommandButton1` is. I'd assume it's an instance of `cButtonClass`, but the name is the default name of a design-time button, so I can't tell what's what just by looking at the code in the OP. – Mathieu Guindon Oct 10 '18 at 20:49
  • 3
    @Berryl - `ObjPtr` is not reliable after it has been wrapped in a `Variant`. You need to use `VarPtr` in that case. `Debug.Assert VarPtr(o.First) = VarPtr(f.CommandButton1)` should pass. – Comintern Oct 10 '18 at 20:58
  • @MathieuGuindon. How would you go about firing the Button.Click event so a unit test wouldn't need to show the form? Happy to make that a separate question if the answer isn't a one-liner... – Berryl Oct 10 '18 at 21:08
  • 1
    Depends what you're testing. If you're testing the form because that's where your logic is, you have a design problem (Smart UI [anti]-pattern) and need to refactor towards a more MVP-like solution. I don't write unit tests for view-level logic, so my one-liner answer would be "you don't". How would you go about doing the exact same thing in C#? You wouldn't, because you wouldn't have the form running the show in C# - so, do the same in VBA! (several articles about that on my blog) – Mathieu Guindon Oct 10 '18 at 21:12
  • no, the logic is in a POCO. just looking for a way to test that the event hooks up properly, which I would do in C# also – Berryl Oct 10 '18 at 21:24
  • Sounds like testing the framework itself to me. But anyway you could have a public `OnClick` method that fires the `Click` event. – Mathieu Guindon Oct 10 '18 at 21:33
  • 1
    Anyway for testing you can make your wrapper class encapsulate the MSForms control with a Private WithEvents field, and then expose your own Click event for the form to handle. If you want to test the wiring-up between the form and the wrapper class without showing the form, you need an OnClick method on the wrapper to fire the event at will, and then the form needs to fire an event (that the presenter would handle) in response to that, e.g. SaveChangesClicked; the test setup then needs a WithEvents form instance that you just don't show. The actual button click can't be unit tested AFAIK. – Mathieu Guindon Oct 10 '18 at 22:50

1 Answers1

2

Here is updated code reflecting the above comments. It works as expected. However, I don't have an answer yet for your other question concerning ObjPtr.

Here's the class code:

Option Explicit

Private WithEvents evFirst As MSForms.CommandButton

Property Get First() As MSForms.CommandButton
    Set First = evFirst
End Property

Property Set First(ByRef o As MSForms.CommandButton)
    Set evFirst = o
End Property

Private Sub evFirst_Click()
    MsgBox "Class Click"
End Sub

Here's the sheet code:

Option Explicit

Public Sub Tester()
    Dim f As UserForm1
    Dim o As cButtonClass

    Set f = New UserForm1
    Set o = New cButtonClass
    Set o.First = f.CommandButton1
    f.Show vbModal

    Dim k1 As LongPtr: k1 = ObjPtr(o.First)
    Dim k2 As LongPtr: k2 = ObjPtr(f.CommandButton1)
    Debug.Assert k1 = k2 'NOPE!
End Sub

Here's the userform code:

Private Sub CommandButton1_Click()
   MsgBox "UserForm Click"
End Sub
Brian M Stafford
  • 8,483
  • 2
  • 16
  • 25