NEWBIE，这是变成意大利面代码吗？

-2

所以我从字面上24小时试图学习这一点。我知道这可以简化为更小的方法和功能，但我还没有那么远。所以这个代码，它的方式工作，我还没有让它崩溃太难。任何帮助和建议都非常感谢。谢谢你们。NEWBIE，这是变成意大利面代码吗？

Private Sub CommandButton2_Click() 

Dim myInOut As String 

Dim myToolType As String 

Dim myOrderNumber As String 

Dim myQTY As Integer 

Dim myToolDesc As String 

Dim myEmployee As String 

Dim myDate As Date 

Dim myLastRowA As Long 

Dim myLastRowB As Long 

Dim myRow As Long 

Dim myNewTool As Long 

Dim myNewRow As Long 

myDate = DateTime.Now 

myLastRowA = ActiveWorkbook.Sheets("Sheet1").Range("A3").End(xlDown).Row - 2 

If ActiveWorkbook.Sheets("Sheet1").Cells(3, 2).Value = "" Then 

    Exit Sub 
End If 


'Finds the Matching Row for the In/Out Order Number AND Updates QTY 

'MsgBox "LastRowA - " & myLastRowA  'Debugging 

For a = 1 To myLastRowA 

    myNewTool = True 
    myLastRowB = ActiveWorkbook.Sheets("Sheet2").Range("A3").End(xlDown).Row 
    'MsgBox "LastRowB - " & myLastRowB & vbCrLf & "OrderNumber - " & myOrderNumber 'Debugging 
     With ActiveWorkbook.Sheets("Sheet1") 
      myInOut = .Cells(3, 1).Value 
      myToolType = .Cells(3, 2).Value 
      myOrderNumber = .Cells(3, 3).Value 
      myQTY = .Cells(3, 4).Value 
      myToolDesc = .Cells(3, 5).Value 
      myEmployee = .Cells(3, 6).Value 

      For b = 1 To myLastRowB 
       If ActiveWorkbook.Sheets("Sheet2").Cells(b, 2).Value = myOrderNumber Then 
        myRow = ActiveWorkbook.Sheets("Sheet2").Cells(b, 2).Row 
        'MsgBox "True " & myRow  'Debugging 
         If myInOut = "IN" Then 
          'Adds In/Out QTY to the Stock QTY 
          ActiveWorkbook.Sheets("Sheet2").Cells(myRow, 4).Value = ActiveWorkbook.Sheets("Sheet2").Cells(myRow, 4).Value + myQTY 
          ActiveWorkbook.Sheets("Sheet1").Rows(3).Delete 
          myNewTool = False 
         ElseIf myInOut = "OUT" Then 
          'Subtracts In/Out QTY to the Stock QTY 
          ActiveWorkbook.Sheets("Sheet2").Cells(myRow, 4).Value = ActiveWorkbook.Sheets("Sheet2").Cells(myRow, 4).Value - myQTY 
          ActiveWorkbook.Sheets("Sheet1").Rows(3).Delete 
          myNewTool = False 
         End If 
       End If 
      Next b 
     If myNewTool = True Then 
      myNewRow = myLastRowB + 1 
      'MsgBox "NewTool" & myNewTool  'Debugging 
      If myInOut = "IN" Then 
       ActiveWorkbook.Sheets("Sheet2").Cells(myNewRow, 1).Value = myToolType 
       ActiveWorkbook.Sheets("Sheet2").Cells(myNewRow, 2).Value = myOrderNumber 
       ActiveWorkbook.Sheets("Sheet2").Cells(myNewRow, 3).Value = myToolDesc 
       ActiveWorkbook.Sheets("Sheet2").Cells(myNewRow, 4).Value = myQTY 
       ActiveWorkbook.Sheets("Sheet1").Rows(3).Delete 
      ElseIf myInOut = "OUT" Then 
       MsgBox "THE ORDER NUMBER OF " & myOrderNumber & " IS NOT IN THE DATABASE" & vbCrLf & myToolType & " " & myToolDesc & vbCrLf & "DOES NOT EXIST. PLEASE EDIT YOUR SELECTION." 
       Exit Sub 
      End If 
     End If 
    End With 
Next a 

End Sub

来源

2016-02-09 Scott Jethro Hardwick

使用工作表中的变量例如'Dim sht2 As WorkSheet：Set sht2 = ActiveWorkbook.Sheets（“Sheet2”）'将减少代码量并使其更易于修改/维护。 –

我投票结束这个问题作为题外话，因为它属于[代码评论]（http://codereview.stackexchange.com/） –

如果你想找到最后一行，最好使用（例如）'' sht2.Cells（rows.Count，1）.End（xlUp）'而不是使用'End（xlDown）'，它会对（1）只有一行（2）有数据空缺 –

这是一个Sub很多功能，所以是的，它趋向于意大利面代码。

人们对Sub应该有多长时间有不同的意见，但作为一个经验法则，您通常希望例程，函数和其他代码单元不超过10-20行;例外情况应该非常罕见。即使删除了评论和空白行，您也有大约60行。那正在起床。你也有十几个变量，嵌套循环和大量的if语句。您的代码的cyclomatic complexity因此变得相当高。

此外，正如评论中指出的那样，您可以多次重复使用多个标识符，例如ActiveWorkbook.Sheets("Sheet1").Range("A3")。这会使代码难以阅读，如果在代码运行时发生任何更改并影响其中一个值，将会导致代码难以破解。

所以，是的，你应该尝试把它分解成小块。

来源

2016-02-09 02:06:37

NEWBIE，这是变成意大利面代码吗？

回答

相关问题