• Hi All

    Please note that at the Chandoo.org Forums there is Zero Tolerance to Spam

    Post Spam and you Will Be Deleted as a User

    Hui...

  • When starting a new post, to receive a quicker and more targeted answer, Please include a sample file in the initial post.

Simplify and speed - VBA Macro

A11 Mighty

Member
Hello - Looking for some assistance to see if there are ways to help simplify and speed the Macros listed below.
This code was developed by a previous colleague.
Attached file for reference.
Also, remove the Unprotect and Protect Active sheet as it's not required.

Add lines command button
Code:
Private Sub Assembly_BOM_Add_Lines_Click()
    ActiveSheet.Unprotect
    Application.ScreenUpdating = False
    
    'Find the end of the BOM section
    Dim r As Integer
    Dim EndRow As Integer
    For r = 12 To 112
        If Sheets("GM Analysis").Range("A" & r).Value = "Total value for of Materials" Then
            EndRow = r - 2
            GoTo A
        End If
    Next r
    MsgBox ("The sheet already has the maximum number of line items.")
    GoTo Z
    
A:  'Find where to insert rows and check to make sure it is in the BOM section
    Dim InsertRow As Integer
    InsertRow = ActiveCell.Row
    If InsertRow < 12 Or InsertRow > EndRow Then
        MsgBox ("Where do you want to insert the row?  Please select a cell in the Bill of Materials section and click the button again.")
        GoTo Z
    End If
    
    'Ask how many rows to insert and check to make sure that it will not cause the BOM section to exceed 100 line items
    Dim MaxRows As Integer
    MaxRows = 100 - EndRow + 11
    Dim NumberRows As Variant
B:  NumberRows = InputBox("How many rows would you like to insert? You can insert up to " & MaxRows & ".")
    If NumberRows = Cancel Then GoTo Z
    If Not NumberRows <= MaxRows Then
        MsgBox ("You have specified too many lines.")
        GoTo B
    End If
      
    'Copy row from Extra Lines tab and insert it into the BOM section
    Dim n As Integer
    For n = 1 To NumberRows
        Sheets("Extra Lines").Rows(2).Copy
        Sheets("GM Analysis").Activate
        ActiveSheet.Rows(InsertRow).Select
        Selection.Insert Shift:=xlDown
    Next n  
Z:
    Application.ScreenUpdating = True
    ActiveSheet.Protect
End Sub

Delete Line
Also, remove the Unprotect and Protect Active sheet as it's not required.

Code:
Private Sub Assembly_BOM_Delete_Line_Click()

    ActiveSheet.Unprotect
    Application.ScreenUpdating = True
    
    'Find the end of the BOM section
    Dim r As Integer
    Dim EndRow As Integer
    For r = 12 To 112
        If Sheets("GM Analysis").Range("A" & r).Value = "Total value for Bill of Materials" Then
            EndRow = r - 2
            GoTo A
        End If
        'MsgBox (Selection.Rows.Count)
    Next r
    MsgBox ("Error: Cannot find the end of the BOM section.")
    GoTo Z

A:  'Loop thru selections in BOM section and ask user if they want to delete each one
    Dim CurrentRow As Integer
    Dim LineItem As Integer
    Dim UserResponse As Variant
        
    For CurrentRow = EndRow To 12 Step -1
        If Not Intersect(Rows(CurrentRow), Selection) Is Nothing Then
            LineItem = Sheets("GM Analysis").Range("A" & CurrentRow)
            UserResponse = MsgBox("You are about to delete line item number " & LineItem & ". You will not be able to undo this action.  Do you want to continue?", vbYesNoCancel)
            If UserResponse = vbCancel Then GoTo Z
            If UserResponse = vbNo Then GoTo B
            ActiveSheet.Rows(CurrentRow).Delete Shift:=xlUp
        End If
B:  Next CurrentRow

Z:
    Application.ScreenUpdating = True
    ActiveSheet.Protect
End Sub
 

Attachments

  • Add & Delete line simplification.xlsm
    98.9 KB · Views: 8
This code cannot be made much simpler but it can be restructured to conform to best programming practices. I made a change that will make it faster but I would be surprised if speed is really a problem to start with since it is not using many rows of data. You would notice this if you were using 10,000 rows instead of 100.

The code you posted does not quite match what is in this file.

Merged cells are usually a problem. I unmerged row 35 for the revised code to work. The text is indented to place it in the same position it was in the merged cells without changing the actual text.

GoTo statements are almost always bad. I restructured the code to eliminate them.

I made it faster by using Find instead of looping through every row.

Cancel was not declared (but it was not defined anyway and not needed). I added Option Explicit to require declarations.
 

Attachments

  • A11 Mighty=Add & Delete line simplification.xlsm
    97.6 KB · Views: 4
A snippet, instead of:
Code:
Dim n As Integer
For n = 1 To NumberRows
  Sheets("Extra Lines").Rows(2).Copy
  'Sheets("GM Analysis").Activate
  Sheets("Chandoo").Activate
  ActiveSheet.Rows(InsertRow).Select
  Selection.Insert Shift:=xlDown
Next n
test:
Code:
Sheets("Extra Lines").Rows(2).Copy
Rows(InsertRow).Resize(NumberRows).Insert
No need for Activesheet since unqualified range references in a sheet's code-module always refer to that sheet (unless of course you want the insert to be in a sheet which is different from the one holding the code (and having the button))

Also in column A of both sheets it's simpler to have
=ROW()-11
rather than
=ROW(A2)-11
because the reference always refers to its own row.
 
This code cannot be made much simpler but it can be restructured to conform to best programming practices. I made a change that will make it faster but I would be surprised if speed is really a problem to start with since it is not using many rows of data. You would notice this if you were using 10,000 rows instead of 100.

The code you posted does not quite match what is in this file.

Merged cells are usually a problem. I unmerged row 35 for the revised code to work. The text is indented to place it in the same position it was in the merged cells without changing the actual text.

GoTo statements are almost always bad. I restructured the code to eliminate them.

I made it faster by using Find instead of looping through every row.

Cancel was not declared (but it was not defined anyway and not needed). I added Option Explicit to require declarations.

Appreciate the help, for some reason the Delete command button doesn't seem to work. I am currently using a friend laptop and that may be due to his security settings. will check later tonight and keep you posted.

Cheers,
A11 Mighty
 
A snippet, instead of:
Code:
Dim n As Integer
For n = 1 To NumberRows
  Sheets("Extra Lines").Rows(2).Copy
  'Sheets("GM Analysis").Activate
  Sheets("Chandoo").Activate
  ActiveSheet.Rows(InsertRow).Select
  Selection.Insert Shift:=xlDown
Next n
test:
Code:
Sheets("Extra Lines").Rows(2).Copy
Rows(InsertRow).Resize(NumberRows).Insert
No need for Activesheet since unqualified range references in a sheet's code-module always refer to that sheet (unless of course you want the insert to be in a sheet which is different from the one holding the code (and having the button))

Also in column A of both sheets it's simpler to have
=ROW()-11
rather than
=ROW(A2)-11
because the reference always refers to its own row.

Much appreciated, I will give it a try later tonight and keep you posted. As always I appreciate your help.

Cheers,
A11 Mighty
 
Back
Top