Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"With inspections" #2995

Open
retailcoder opened this issue Apr 28, 2017 · 5 comments
Open

"With inspections" #2995

retailcoder opened this issue Apr 28, 2017 · 5 comments
Labels
difficulty-02-ducky Resolving these involves the internal API, but with relatively easy problems to solve. enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope. feature-inspections up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky

Comments

@retailcoder
Copy link
Member

With Me

I can't think of a single valid use case for With Me:

With Me
    .Controls(ctrlName).Value = 42
End With

Can be rewritten as:

Controls(ctrlName).Value = 42

With Me hides the non-public members because it accesses the current instance from its public interface, which is inviting hybrid code referencing private members from the current instance, and public members from the current instance's public interface: that's certainly making the VBA runtime work much harder than it needs to, even with everything being early-bound.

With UserForm1

This one is actually a bug waiting to happen:

With UserForm1
     .Controls(ctrlName).Value = 42
End With

If that's in the code-behind of UserForm1, then it's working against the form's default instance, which may or may not be the current instance, so With Me would be the intention... which means the code is better off as, again:

Controls(ctrlName).Value = 42

That's applicable to any class with a default instance, not just forms.

@retailcoder retailcoder added difficulty-02-ducky Resolving these involves the internal API, but with relatively easy problems to solve. feature-inspections up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky labels Apr 28, 2017
@retailcoder
Copy link
Member Author

Okay, I just thought of a legit use case for With Me:

With Me
    lastRow = .Range("A1:A" & .Cells(.Rows.Count, 1)).End(xlUp).Row
    '...
End With

That is, if the module is a document module, With Me is actually preventing ambiguous code and enhancing readability in that case. The unqualified alternative triggers implicit ActiveSheet reference inspection results.

@Vogel612 Vogel612 added the enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope. label Nov 26, 2017
@retailcoder
Copy link
Member Author

Semi-related, should there be a refactoring for inlining With statements? Or an(other?) inspection to flag With blocks that work off an existing local variable?

@comintern
Copy link
Contributor

The main problem that I would see with inlining With statements is that the expression that captures the With variable might have side effects. For example:

    With ThisWorkbook.Worksheets.Add
        .Name = "NewSheet"
        ' ...
    End With

In that case, the refactoring would need to introduce a local variable. The problem with that, is that End With can also have side effects.

With DefaultInstance
    .Foo = 42
    .Bar = "bar"
End With    '<-- default instance is set to Nothing.

In-lining that requires an explicit Set x = Nothing call, which leaves a bad taste in my mouth.

@retailcoder
Copy link
Member Author

The first case is trivially solved though; the With expression is just brought to RHS of the new variable's assignment (assuming we can correctly get the Set keyword requirement for it... which is a whole other issue). As for default instances... snap.

@bclothier
Copy link
Contributor

FWIW, I cannot agree with the idea of replacing Me.Controls(ctrlName) with just Controls(ctrlName) because that leads to ambiguity. Here's an example to illustrate the problem:

Private Sub Form_Current()
    Dim Text0 As Variant

    Text0 = "hello, world!"
    Me.Text0 = "goodbye, world!"

    Debug.Print Text0, Me.Text0
End Sub

In a more complex module, especially with module level variables, that can be easily missed and can lead to ambiguous name. I'd rather that people were trained to consistently Me. any members of the userform/document modules, as that usually promotes readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty-02-ducky Resolving these involves the internal API, but with relatively easy problems to solve. enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope. feature-inspections up-for-grabs Use this label in conjunction with a difficulty level label, e.g. difficulty-02-ducky
Projects
None yet
Development

No branches or pull requests

4 participants