weixin_39753397
weixin_39753397
2020-12-30 03:19

"Variable can be passed ByVal" inspection incorrectly raised.

I have Method (A) which receives a parameter, but doesn't explicitly change that parameter. Instead, it passes it to Method (B) which changes the value of that parameter. In both methods, I had the parameter declared ByRef.

Code Inspections identified the parameter to Method (A) as a candidate to be changed from ByRef to ByVal. Without thorough investigation, I allowed it to fix it, then later realized the bug when Method (A) no longer returned the changed value for the parameter.

Example Code:


Private Sub PrintDataRows(ByVal Report As Worksheet, ByRef AllClinicRow As Integer, ByRef AllClinicStartRow As Integer, ByRef NoStateRow As Integer, ByRef NoStateStartRow As Integer, _
                  ByRef NoStatePageBreak As Integer, ByVal ClinicList As ProcessClinic)

Dim CI As ClinicItem
Const ClinicsToSkip As String = "State of Indiana"

  For Each CI In ClinicList.ClinicItems
    Application.StatusBar = "Printing: " & CI.Name
    DoEvents
    PrintDataRow AllClinicRow, AllClinicStartRow, Report, ClinicList, CI.Name, psPrintState
    IncAll Report, AllClinicRow, NoStateRow, NoStateStartRow, NoStatePageBreak
    If InStr(1, ClinicsToSkip, CI.Name) = 0 Then      'If the clinic isn't one to be skipped, print it
      PrintDataRow NoStateRow, NoStateStartRow, Report, ClinicList, CI.Name, psDoNotPrintState
      IncNoState Report, AllClinicRow, NoStateRow
    End If
  Next

End Sub


Private Sub IncAll(ws As Worksheet, ByRef Clinic As Integer, ByRef State As Integer, ByRef NoStateStartRow As Integer, ByRef PageBreakRow As Integer)

  Clinic = Clinic + 1           'prepare for next row of all-clinic data
  Rows(Clinic + 1).EntireRow.Insert 'insert a new blank row
  'increment all the other pointers to account for increased table sizes
  State = State + 1
  NoStateStartRow = NoStateStartRow + 1
  PageBreakRow = PageBreakRow + 1

End Sub

该提问来源于开源项目:rubberduck-vba/Rubberduck

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

5条回答

  • weixin_39629093 weixin_39629093 4月前

    haven't forgotten about this issue.

    Quite a lot of little things had to be improved for this to work, but as with all improvements, the result is an even more powerful API! :smile:

    So...

    When called parameter is passed by value, the ByRef parameter being passed there can be passed by value if it's a "primitive type":

    image

    When called parameter is passed by reference, the ByRef parameter being passed there can't be passed by value without breaking the code, so there's no inspection result for it:

    image

    Same if the ByRef parameter is passed to a ByRef parameter that's assigned - except now neither foo nor bar can be passed by value here:

    image

    Rubberduck will not suggest to pass Variant and object types by value.

    See commit 580fcca235eb877a6e2fc0e676d8154b24ef3aad on my fork; this issue will be closed when I merge my changes in.

    点赞 评论 复制链接分享
  • weixin_39637571 weixin_39637571 4月前

    Hmm... Should we open a separate issue to add this inspection for object types?

    点赞 评论 复制链接分享
  • weixin_39629093 weixin_39629093 4月前

    no need. The 'primitive types only' part of the spec is nothing new, and it was argued that objects passed by reference shouldn't be quickfixed to byval, for performance reasons. I would be more than happy to remove all this additional logic!

    点赞 评论 复制链接分享
  • weixin_39637571 weixin_39637571 4月前

    Seems like that should be configurable to me. Turn it off if you want to. Personally, I pass everything ByVal (including objects) for the safety factor, but it is true that there's a performance consideration there.

    Your call.

    点赞 评论 复制链接分享
  • weixin_39629093 weixin_39629093 4月前

    The fix for this involves verifying whether a parameter is passed directly into another method that accepts it ByRef, effectively treating that operation as an assignment. We need to think of a way to implement this without impacting performance too much.

    点赞 评论 复制链接分享

相关推荐