文章/答案/技术大牛

发布

社区首页 >问答首页 >继承代码- Worksheet_Change事件代码

问继承代码- Worksheet_Change事件代码
EN

Code Review用户

提问于 2019-11-26 14:04:10

回答 2查看 98关注 0票数 3

我继承了这段代码，必须修复它。它确实有效，我知道我可以使用If Not Intersect(Target, Me.Range()) Is Nothing Then语法重构代码。我想知道在本例中使用函数传递单元格引用是否最好，但我还不太熟悉函数的使用，我希望通过下面的代码提供一些关于最佳实践的输入或指导。请注意，我很清楚select在这个代码块中的使用情况，但是最初的作者希望我保持select能够根据工作表中的选择移动活动单元格。

Private Sub Worksheet_Change(ByVal Target As Range)

    Application.ScreenUpdating = False
    Application.EnableEvents = False

    Dim wb As Workbook: Set wb = Application.ThisWorkbook
    Dim wsDE As Worksheet: Set wsDE = wb.Sheets("Data Entry")

    Dim Unique_Identifier As String
    Dim Wire_Type As String

    With wsDE
        Select Case Target.Address
            Case Is = "$B$4": Hide_All
                Select Case Range("B4")
                    Case Is <> ""
                        Range("A100:A199").EntireRow.Hidden = False
                        Range("B101").Select
                        Sheet5.Visible = xlSheetVisible 'Confirmation-Incoming
                        Range("B5") = ""
                    Case Else: Range("B5").Select
                End Select
            Case Is = "$B$5": Hide_All
                Select Case Range("B5")
                    Case Is <> ""
                        Range("A200:A211").EntireRow.Hidden = False
                        Range("A216:A227").EntireRow.Hidden = False
                        Range("B201").Select
                        With ThisWorkbook
                            Sheet7.Visible = xlSheetVisible 'Checklist
                            Sheet4.Visible = xlSheetVisible 'Confirmation-Outgoing-1
                            Sheet2.Visible = xlSheetVisible 'Wire Transfer Request-1
                        End With

                        Select Case Range("B5")
                            Case Is > 1
                                Range("A200:A299").EntireRow.Hidden = False
                                Unique_Identifier = Range("B5").Value
                                Wire_Type = "Deposit/Loan"
                                Call Find_Recurring(Unique_Identifier, Wire_Type)
                        End Select

                    Case Else: Range("B6").Select
                End Select
            Case Is = "$B$6": Hide_All
                Select Case Range("B6")
                    Case Is <> ""
                        Range("A300:A312").EntireRow.Hidden = False
                        Range("A316:A330").EntireRow.Hidden = False
                        Range("B301").Select
                        With ThisWorkbook
                            Sheet3.Visible = xlSheetVisible  'Checklist-Loan Closing
                            Sheet12.Visible = xlSheetVisible 'Confirmation-Outgoing-2
                            Sheet11.Visible = xlSheetVisible 'Wire Transfer Request-2
                        End With
                    Case Else: Range("B7").Select
                End Select
            Case Is = "$B$7": Hide_All
                Select Case Range("B7")
                    Case Is <> ""
                        Range("A400:A411").EntireRow.Hidden = False
                        Range("A414:A499").EntireRow.Hidden = False
                        Range("B401").Select
                        With ThisWorkbook
                            Sheet9.Visible = xlSheetVisible  'Checklist-Cash Management
                            Sheet14.Visible = xlSheetVisible 'Confirmation-Outgoing-3
                        End With
                    Case Else: Range("B8").Select
                End Select
            Case Is = "$B$8": Hide_All
                Select Case Range("B8")
                    Case Is <> ""
                        Range("A500:A599").EntireRow.Hidden = False
                        Range("B501").Select
                        With ThisWorkbook
                            Sheet13.Visible = xlSheetVisible 'Wire Transfer Request - Brokered-Internet
                        End With
                    Case Else: Range("B9").Select
                End Select
            Case Is = "$B$9": Hide_All
                Select Case Range("B9")
                    Case Is <> ""
                        Range("A600:A610").EntireRow.Hidden = False
                        Range("B601").Select
                        Sheet8.Visible = xlSheetVisible 'Checklist-Internal

                        Select Case Range("B9")
                            Case Is > 1
                                Range("A600:A699").EntireRow.Hidden = False
                                Unique_Identifier = Range("B9").Value
                                Wire_Type = "Internal"
                                Call Find_Recurring(Unique_Identifier, Wire_Type)
                        End Select

                    Case Else: Range("B10").Select
                End Select
            Case Is = "$B$10": Hide_All
                Select Case Range("B10")
                    Case Is <> ""
                        Sheet6.Visible = xlSheetVisible 'Wire Transfer Agreement
                        Sheets("Wire Transfer Agreement").Visible = True
                        Range("A5000:A5099").EntireRow.Hidden = False
                        Range("A5005:A5011").EntireRow.Hidden = True
                        Range("B5001").Select
                    Case Else: Range("B11").Select
                End Select
            Case Is = "$B$11": Hide_All
                Select Case Range("B11")
                    Case Is <> ""
'                        Sheets("Recurring Wire Transfer Request").Visible = True
                        Sheet18.Visible = xlSheetVisible 'Recurring Wire Transfer Request
                        Range("A5100:A5118").EntireRow.Hidden = False
                        Range("A5111:A5114").EntireRow.Hidden = True
                        Range("B5101").Select
                    Case Else: Range("B11").Select
                End Select


    'Wires from Deposit Account or Loan (Post-Closing) Section
            Case Is = "$B$205"
                Select Case LCase(Range("B205"))
                    Case Is = "yes"
                        Range("A212:A215").EntireRow.Hidden = False
                    Case Else
                        Range("A212:A215").EntireRow.Hidden = True
                        Range("B206").Select
                End Select
            Case Is = "$B$227"
                Select Case LCase(Range("B227"))
                    Case Is = "domestic"
                        Range("A222:A243").EntireRow.Hidden = False
                        Range("A267:A299").EntireRow.Hidden = False
                        Range("A244:A266").EntireRow.Hidden = True
                        Range("B229").Select
                    Case Is = "international"
                        Range("A244:A299").EntireRow.Hidden = False
                        Range("A228:A243").EntireRow.Hidden = True
                        Range("B245").Select
                    Case Is <> "international", "domestic"
                        Range("A228:A299").EntireRow.Hidden = True
                        Range("B227").Select
                End Select
            Case Is = "$B$269"
                Select Case LCase(Range("B269"))
                    Case Is = "yes"
                        Sheets("Wire Transfer Agreement").Visible = True
                        Range("A5000:A5099").EntireRow.Hidden = False
                        Range("B282:B299").EntireRow.Hidden = True
                        Application.ScreenUpdating = True
                        Range("B5001").Select
                    Case Else
                        Sheets("Wire Transfer Agreement").Visible = False
                        Range("A5000:A5099").EntireRow.Hidden = True
                        Range("B281:B299").EntireRow.Hidden = False
                        Range("B270").Select
                End Select

    'Loan-Closing Wires Section
            Case Is = "$B$306"
                Select Case LCase(Range("B306"))
                    Case Is = "yes"
                        Range("A313:A316,A331").EntireRow.Hidden = False
                    Case Else
                        Range("A313:A316").EntireRow.Hidden = True
                        Range("A331").EntireRow.Hidden = False
                        Range("B307").Select
                End Select
            Case Is = "$B$331"
                Select Case LCase(Range("B331"))
                    Case Is = "domestic"
                        Range("A332:A347").EntireRow.Hidden = False
                        Range("A370:A399").EntireRow.Hidden = False
                        Range("A348:A369").EntireRow.Hidden = True
                        Range("B331").Select
                    Case Is = "international"
                        Range("A347:A399").EntireRow.Hidden = False
                        Range("A332:A346").EntireRow.Hidden = True
                        Range("B349").Select
                    Case Is <> "domestic", "international"
                        Range("A332:A399").EntireRow.Hidden = True
                        Range("B331").Select
                End Select
            Case Is = "$B$373"
                Select Case LCase(Range("B373"))
                    Case Is = "yes"
                        Sheets("Wire Transfer Agreement").Visible = True
                        Range("A5000:A5099").EntireRow.Hidden = False
                        Range("B383:B399").EntireRow.Hidden = True
                        Application.ScreenUpdating = True
                        Range("B5001").Select
                    Case Else
                        Sheets("Wire Transfer Agreement").Visible = False
                        Range("A5000:A5099").EntireRow.Hidden = True
                        Range("B383:B399").EntireRow.Hidden = False
                        Range("B374").Select
                End Select

    'Cash Management Wires Section
            Case Is = "$B$406"
                Select Case LCase(Range("B406"))
                    Case Is = "yes"
                        Range("A412:A413").EntireRow.Hidden = False
                    Case Else
                        Range("A412:A413").EntireRow.Hidden = True
                        Range("B407").Select
                End Select

            Case Is = "$B$425"
                Select Case LCase(Range("B425"))
                    Case Is = "yes"
                        Range("A430:A431").EntireRow.Hidden = False
                    Case Else
                        Range("A430:A431").EntireRow.Hidden = True
                        Range("B426").Select
                End Select


    'Internal Foresight Wires Section
            Case Is = "$B$610"
                Select Case LCase(Range("B610"))
                    Case Is = "domestic"
                        Range("A611:A625").EntireRow.Hidden = False
                        Range("A648:A699").EntireRow.Hidden = False
                        Range("A626:A647").EntireRow.Hidden = True
                        Range("B612").Select
                    Case Is = "international"
                        Range("A626:A699").EntireRow.Hidden = False
                        Range("A611:A625").EntireRow.Hidden = True
                        Range("B627").Select
                    Case Is <> "international", "domestic"
                        Range("A611:A699").EntireRow.Hidden = True
                        Range("B610").Select
                End Select

    'Wire Transfer Agreement Section
            Case Is = "$B$5004"
                Range("A5005:A5011").EntireRow.Hidden = True
                Range("B5004").Select
                Select Case LCase(Range("B5004"))
                    Case Is = "entity"
                        Range("A5007:A5011").EntireRow.Hidden = False
                        Range("B5007").Select
                    Case Is = "individual(s)"
                        Range("A5005:A5006").EntireRow.Hidden = False
                        Range("B5005").Select
                End Select

    'Recurring Wire Transfer Request Section

            Case Is = "$B$5104"
                Range("A5111:A5114").EntireRow.Hidden = True
                Range("B5105").Select
                Select Case LCase(Range("B5104"))
                    Case Is = "yes"
                        Range("A5111:A5114").EntireRow.Hidden = False
                        Range("B5105").Select
                    Case Is = "no"
                        Range("A5111:A5114").EntireRow.Hidden = True
                        Range("B5105").Select
                End Select

            Case Is = "$B$5118"
                Select Case LCase(Range("B5118"))
                    Case Is = "domestic"
                        Range("A5119:A5131").EntireRow.Hidden = False
                        Range("A5132:A5199").EntireRow.Hidden = True
                        Range("A5150").EntireRow.Hidden = False
                        Range("B5120").Select
                    Case Is = "international"
                        Range("A5119:A5131").EntireRow.Hidden = True
                        Range("A5132:A5149").EntireRow.Hidden = False
                        Range("A5151:A5199").EntireRow.Hidden = True
                        Range("B5133").Select
                    Case Is <> "international", "domestic"
                        Range("A5119:A5199").EntireRow.Hidden = True
                        Range("B5118").Select
                End Select
        End Select

    End With



'CIF Calls
    If Not Intersect(Target, Range("B103")) Is Nothing Then CIFIncoming
    If Not Intersect(Target, Range("B206")) Is Nothing Then CIFOutD
    If Not Intersect(Target, Range("B307")) Is Nothing Then CIFOutL
    If Not Intersect(Target, Range("B407")) Is Nothing Then CIFOutCM
    If Not Intersect(Target, Range("B506")) Is Nothing Then CIFBrokered

    Application.ScreenUpdating = True
    Application.EnableEvents = True


End Sub

vba

excel

回答 2

Code Review用户

回答已采纳

发布于 2019-11-27 12:25:07

正如AJD所指出的那样，使用命名Range将使代码更容易理解、读、写和修改。同样的逻辑可以应用于工作表代码名。

下面是我在重构代码时使用的名称：

单张(“电汇协议”) -> wsWTA
Sheet2 -> wsWireTransferRequest1
Sheet3 -> wsChecklistLoanClosing
Sheet4 -> wsConfirmationOutgoing1
Sheet5 -> wsConfirmationIncoming
Sheet7 -> wsChecklist
Sheet6 -> wsWireTransferAgreement
Sheet8 -> wsChecklistInternal
Sheet9 -> wsChecklistCashManagement
Sheet11 -> wsWireTransferRequest2
Sheet12 -> wsConfirmationOutgoing2
Sheet13 -> wsWTRBrokeredInternet
Sheet14 -> wsConfirmationOutgoing3
Sheet18 -> wsRecurringWTR

使用嵌套的Select语句特别难读。通常，我会将Select与If..ElseIf..Else语句交替使用，但是过程太长；所以我建议为顶级Select语句的每个Case编写一个子例程(参见下面的代码)。

我只在使用Range.EntireRow变量(例如Target.EntireRow)时使用Range。直接使用Rows()将使您的代码更加简洁，而额外的空白将使您更容易阅读。

在此之前

范围(“A 200:A 211”).EntireRow.Hidden= False后行(“200:211”).Hidden= False

不再需要Application.ScreenUpdating = True。ScreenUpdating现在在代码执行完毕后恢复。

重构代码

Private Sub Worksheet_Change(ByVal Target As Range)
    Application.ScreenUpdating = False
    Application.EnableEvents = False
    Dim Unique_Identifier As String
    Dim Wire_Type As String

    Select Case Target.Address
        Case Is = "$B$4"
            EntryB4
        Case Is = "$B$5"
            EntryB5
        Case Is = "$B$6"
            EntryB6
        Case Is = "$B$7"
            EntryB7
        Case Is = "$B$8"
            EntryB8
        Case Is = "$B$9"
            EntryB9
        Case Is = "$B$10"
            EntryB10
        Case Is = "$B$11"
            EntryB11
            Rem Wires from Deposit Account or Loan (Post-Closing) Section
        Case Is = "$B$205"
            EntryB205
        Case Is = "$B$227"
            EntryB227
        Case Is = "$B$269"
            EntryB269
            Rem Loan-Closing Wires Section
        Case Is = "$B$306"
            EntryB306
        Case Is = "$B$331"
            EntryB331
        Case Is = "$B$373"
            EntryB373
            Rem Cash Management Wires Section
        Case Is = "$B$406"
            EntryB406
        Case Is = "$B$425"
            EntryB425
            Rem Internal Foresight Wires Section
        Case Is = "$B$610"
            EntryB610
            Rem Wire Transfer Agreement Section
        Case Is = "$B$5004"
            EntryB5004
            Rem Recurring Wire Transfer Request Section
        Case Is = "$B$5104"
            EntryB5104
        Case Is = "$B$5118"
            EntryB5118
    End Select

    Rem CIF Calls
    If Not Intersect(Target, Range("B103")) Is Nothing Then CIFIncoming
    If Not Intersect(Target, Range("B206")) Is Nothing Then CIFOutD
    If Not Intersect(Target, Range("B307")) Is Nothing Then CIFOutL
    If Not Intersect(Target, Range("B407")) Is Nothing Then CIFOutCM
    If Not Intersect(Target, Range("B506")) Is Nothing Then CIFBrokered

    Application.EnableEvents = True

End Sub

Sub EntryB4()
    With wsDataEntry
        .Activate
        Hide_All
        Select Case .Range("B4")
            Case Is <> ""
                .Rows("100:199").Hidden = False
                .Range("B101").Select
                wsConfirmationIncoming.Visible = xlSheetVisible
                .Range("B5") = ""
            Case Else
                .Range("B5").Select
        End Select
    End With
End Sub

Sub EntryB5()
    With wsDataEntry
        Hide_All
        .Activate
        Select Case .Range("B5")
            Case Is <> ""
                .Rows("200:211").Hidden = False
                .Rows("216:227").Hidden = False
                .Range("B201").Select
                With ThisWorkbook
                    wsChecklist.Visible = xlSheetVisible
                    wsConfirmationOutgoing1.Visible = xlSheetVisible
                    wsWireTransferRequest1.Visible = xlSheetVisible
                End With

                Select Case .Range("B5")
                    Case Is > 1
                        .Rows("200:299").Hidden = False
                        Unique_Identifier = .Range("B5").Value
                        Wire_Type = "Deposit/Loan"
                        Call Find_Recurring(Unique_Identifier, Wire_Type)
                End Select

            Case Else: .Range("B6").Select
        End Select
    End With
End Sub

Sub EntryB7()
    With wsDataEntry
        Hide_All
        .Activate
        Select Case .Range("B7")
            Case Is <> ""
                .Rows("400:411").Hidden = False
                .Rows("414:499").Hidden = False
                .Range("B401").Select
                With ThisWorkbook
                    wsChecklistCashManagement.Visible = xlSheetVisible
                    wsConfirmationOutgoing3.Visible = xlSheetVisible
                End With
            Case Else: .Range("B8").Select
        End Select
    End With
End Sub

Sub EntryB8()
    With wsDataEntry
        Hide_All
        .Activate
        Select Case .Range("B8")
            Case Is <> ""
                .Rows("500:599").Hidden = False
                .Range("B501").Select
                With ThisWorkbook
                    wsWTRBrokeredInternet.Visible = xlSheetVisible
                End With
            Case Else: .Range("B9").Select
        End Select
    End With
End Sub

Sub EntryB9()
    With wsDataEntry
        Hide_All
        .Activate
        Select Case .Range("B9")
            Case Is <> ""
                .Rows("600:610").Hidden = False
                .Range("B601").Select
                wsChecklistInternal.Visible = xlSheetVisible

                Select Case .Range("B9")
                    Case Is > 1
                        .Rows("600:699").Hidden = False
                        Unique_Identifier = .Range("B9").Value
                        Wire_Type = "Internal"
                        Call Find_Recurring(Unique_Identifier, Wire_Type)
                End Select

            Case Else: .Range("B10").Select
        End Select
    End With
End Sub

Sub EntryB10()
    With wsDataEntry
        Hide_All
        .Activate
        Select Case .Range("B10")
            Case Is <> ""
                Sheet6.Visible = xlSheetVisible
                wsWTA.Visible = True
                .Rows("5000:5099").Hidden = False
                .Rows("5005:5011").Hidden = True
                .Range("B5001").Select
            Case Else: .Range("B11").Select
        End Select
    End With
End Sub

Sub EntryB11()
    With wsDataEntry
        Hide_All
        .Activate
        Select Case .Range("B11")
            Case Is <> ""
                wsRecurringWTR.Visible = xlSheetVisible
                .Rows("5100:5118").Hidden = False
                .Rows("5111:5114").Hidden = True
                .Range("B5101").Select
            Case Else: .Range("B11").Select
        End Select
    End With
End Sub

Rem Wires from Deposit Account or Loan (Post-Closing) Section
Sub EntryB205()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B205"))
            Case Is = "yes"
                .Rows("212:215").Hidden = False
            Case Else
                .Rows("212:215").Hidden = True
                .Range("B206").Select
        End Select
    End With
End Sub

Sub EntryB227()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B227"))
            Case Is = "domestic"
                .Rows("222:243").Hidden = False
                .Rows("267:299").Hidden = False
                .Rows("244:266").Hidden = True
                .Range("B229").Select
            Case Is = "international"
                .Rows("244:299").Hidden = False
                .Rows("228:243").Hidden = True
                .Range("B245").Select
            Case Is <> "international", "domestic"
                .Rows("228:299").Hidden = True
                .Range("B227").Select
        End Select
    End With
End Sub

Sub EntryB269()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B269"))
            Case Is = "yes"
                wsWTA.Visible = True
                .Rows("5000:5099").Hidden = False
                .Rows("282:299").Hidden = True

                .Range("B5001").Select
            Case Else
                wsWTA.Visible = False
                .Rows("5000:5099").Hidden = True
                .Rows("281:299").Hidden = False
                .Range("B270").Select
        End Select
    End With
End Sub

Rem Loan-Closing Wires Section
Sub EntryB306()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B306"))
            Case Is = "yes"
                .Range("A313:A316,A331").EntireRow.Hidden = False
            Case Else
                .Rows("313:316").Hidden = True
                .Rows(331).Hidden = False
                .Range("B307").Select
        End Select
    End With
End Sub

Sub EntryB331()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B331"))
            Case Is = "domestic"
                .Rows("332:347").Hidden = False
                .Rows("370:399").Hidden = False
                .Rows("348:369").Hidden = True
                .Range("B331").Select
            Case Is = "international"
                .Rows("347:399").Hidden = False
                .Rows("332:346").Hidden = True
                .Range("B349").Select
            Case Is <> "domestic", "international"
                .Rows("332:399").Hidden = True
                .Range("B331").Select
        End Select
    End With
End Sub

Sub EntryB373()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B373"))
            Case Is = "yes"
                wsWTA.Visible = True
                .Rows("5000:5099").Hidden = False
                .Rows("383:399").Hidden = True

                .Range("B5001").Select
            Case Else
                wsWTA.Visible = False
                .Rows("5000:5099").Hidden = True
                .Rows("383:399").Hidden = False
                .Range("B374").Select
        End Select

    End With
End Sub

Rem Cash Management Wires Section
Sub EntryB406()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B406"))
            Case Is = "yes"
                .Rows("412:413").Hidden = False
            Case Else
                .Rows("412:413").Hidden = True
                .Range("B407").Select
        End Select
    End With
End Sub

Sub EntryB425()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B425"))
            Case Is = "yes"
                .Rows("430:431").Hidden = False
            Case Else
                .Rows("430:431").Hidden = True
                .Range("B426").Select
        End Select
    End With
End Sub

Rem Internal Foresight Wires Section
Sub EntryB610()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B610"))
            Case Is = "domestic"
                .Rows("611:625").Hidden = False
                .Rows("648:699").Hidden = False
                .Rows("626:647").Hidden = True
                .Range("B612").Select
            Case Is = "international"
                .Rows("626:699").Hidden = False
                .Rows("611:625").Hidden = True
                .Range("B627").Select
            Case Is <> "international", "domestic"
                .Rows("611:699").Hidden = True
                .Range("B610").Select
        End Select

    End With
End Sub

Rem Wire Transfer Agreement Section
Sub EntryB5004()
    With wsDataEntry
        .Activate
        .Rows("5005:5011").Hidden = True
        .Range("B5004").Select
        Select Case LCase(.Range("B5004"))
            Case Is = "entity"
                .Rows("5007:5011").Hidden = False
                .Range("B5007").Select
            Case Is = "individual(s)"
                .Rows("5005:5006").Hidden = False
                .Range("B5005").Select
        End Select
    End With
End Sub

Rem Recurring Wire Transfer Request Section
Sub EntryB5104()
    With wsDataEntry
        .Activate
        .Rows("5111:5114").Hidden = True
        .Range("B5105").Select
        Select Case LCase(.Range("B5104"))
            Case Is = "yes"
                .Rows("5111:5114").Hidden = False
                .Range("B5105").Select
            Case Is = "no"
                .Rows("5111:5114").Hidden = True
                .Range("B5105").Select
        End Select
    End With
End Sub

Sub EntryB5118()
    With wsDataEntry
        .Activate
        Select Case LCase(.Range("B5118"))
            Case Is = "domestic"
                .Rows("5119:5131").Hidden = False
                .Rows("5132:5199").Hidden = True
                .Rows(5150).Hidden = False
                .Range("B5120").Select
            Case Is = "international"
                .Rows("5119:5131").Hidden = True
                .Rows("5132:5149").Hidden = False
                .Rows("5151:5199").Hidden = True
                .Range("B5133").Select
            Case Is <> "international", "domestic"
                .Rows("5119:5199").Hidden = True
                .Range("B5118").Select
        End Select
    End With
End Sub

票数 2

Code Review用户

发布于 2019-11-26 20:16:02

我想，在橄榄球比赛中，这就是他们所说的“医院通行证”。

作为一个已经修复了很多这样的代码的人，我只想点击一些亮点。如果你设法修复这些亮点，我希望在第二轮的另一个问题中看到修改后的代码。因为你所做的工作要经过很多次才能得到正确的结果(但这是值得的)。

为自己的成功做好准备

但原作者想让我保留选择

如果您正在修复和维护代码，那么它的编写方式必须使您的易于维护。如果您正在修复这个和其他人必须维护它，然后成为一个好的编码器，并使其易于维护。但是，移动用户查看工作流程中的特定单元是用户的要求，您应该记住这一点。

Option Explicit位于每个模块的顶部。只是提醒一下-你可能已经有了。

在工作表中使用Named范围。这将使未来的维护变得如此容易。它将使当前代码更易于理解-- .Range("DateEntryDate")比.Range("$B$3")更容易理解。

提前退出

仅出于性能原因，始终在启动和退出时找出不运行代码的原因。目前，如果我在$ABC$678023983中做了一个更改，这段代码就会启动并运行。真是浪费时间和周期！示例：

Private Sub Worksheet_Change(ByVal Target As Range)
Dim validRange as Range
    Set validRange = Intersect(Target, Me.Range("$B4:$B11")) '<-- wow, a named range here would be good.
    If validRange is Nothing Then
        Exit Sub ' <--- explicit exit, easy to see.
    End If

    If Target.Cells.Count > 1 Then
        Exit Sub ' <-- always manage the range size!
    End If

    ' … other code 
End Sub

限定您的范围

大多数代码都是以不限定的范围编写的(活动工作表上隐含的操作)。

Range("A400:A411").EntireRow.Hidden = False

但是，这假设活动工作表仍然是发生_Change事件的工作表。永远不要做出那样的假设。还记得这个密码吗？

Range("B101").Select

这意味着活动单元将跳转。随着将来对代码或工作簿的修改，这甚至可能会跳转到另一个工作表。

此外，代码还调用一些实用程序函数(例如Hide_all) --这些函数也可能更改活动表。

注意到这一点之后，With wsDE是怎么回事？有一个完整的With块，它根本不引用对象(wsDE)！

码可读性

不要使用接线器，它会导致混乱：

        Case Is = "$B$4": Hide_All
            Select Case Range("B4")
                Case Is <> ""
                    Range("A100:A199").EntireRow.Hidden = False
                    Range("B101").Select
                    Sheet5.Visible = xlSheetVisible 'Confirmation-Incoming
                    Range("B5") = ""
                Case Else: Range("B5").Select
            End Select

应：

        Case Is = "$B$4"
            Hide_All
            Select Case Range("B4")
                Case Is <> ""
                    Range("A100:A199").EntireRow.Hidden = False
                    Range("B101").Select
                    Sheet5.Visible = xlSheetVisible 'Confirmation-Incoming
                    Range("B5") = ""
                Case Else
                    Range("B5").Select
            End Select

突然之间，缩进级别和代码范围更容易理解。Else变得更加明显。读起来容易多了。

声明变量更接近您将要使用它们的位置，并且在相同的范围内。

Dim Unique_Identifier As String
Dim Wire_Type As String

我花了一段时间才知道它们是否真的被使用了。

下一步是什么？

如果您解决上述问题，您将得到一些稍微干净的代码。你会意识到这需要更多的工作。然而，您将有一个更干净的基础来找出剩余的低效率。一步一步，你就会到达那里！

票数 5

页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持

原文链接：

https://codereview.stackexchange.com/questions/232999

复制

相似问题

问继承代码- Worksheet_Change事件代码
EN

回答 2

Code Review用户

重构代码

Code Review用户

为自己的成功做好准备

提前退出

限定您的范围

码可读性

下一步是什么？

社区

活动

圈层

关于

腾讯云开发者

热门产品

热门推荐

更多推荐

问继承代码- Worksheet_Change事件代码EN

回答 2

Code Review用户

重构代码

Code Review用户

为自己的成功做好准备

提前退出

限定您的范围

码可读性

下一步是什么？

社区

活动

圈层

关于

腾讯云开发者

热门产品

热门推荐

更多推荐

问继承代码- Worksheet_Change事件代码
EN