double click on userform running slow excel vba

Question

I have this code for picking items with multiple values from userform and inserting them into a sheet. The code works perfect on my computer, which is pretty decent. But on a computer from work it runs very slowly, maybe because I'm abusing the "select" method. I ran out of ideas.

Sub LSTART_DblClick(ByVal Cancel As MSForms.ReturnBoolean) 
 Application.ScreenUpdating = False
 On Error GoTo ERR1:
 L = LSTART.List(LSTART.ListIndex, 0)
 Sheets("CONCAT").Select
 Range("A2").Select
 While ActiveCell.Value <> "" And ActiveCell.Value <> Val(L) And ActiveCell.Value <> L
 ActiveCell.Offset(1, 0).Select
 Wend
 ActiveCell.Offset(0, 2).Select 
 CODART = ActiveCell.Value
 ActiveCell.Offset(0, 1).Select 
 CODCLR = ActiveCell.Value
 ActiveCell.Offset(0, 1).Select 
 TALLE = ActiveCell.Value
 Sheets("ventas").Select
 Range("B11").Select 
 While ActiveCell.Value <> ""
 ActiveCell.Offset(1, 0).Select 
 Wend 
 
 ActiveCell.Value = CODART
 ActiveCell.Offset(0, 1).Value = CODCLR 
 ActiveCell.Offset(0, 2).Value = TALLE 
 ActiveCell.Offset(1, 0).Select 
ERR1:
Application.ScreenUpdating = True 
End Sub

• Toby Speight: The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles.

Answer 1

Here are some suggestions to optimize your code:

Avoid Selecting Ranges: Instead of selecting ranges and then working with them, directly reference the ranges and cells. This helps to avoid the overhead associated with selecting cells.

Replace:

Sheets("CONCAT").Select
Range("A2").Select

with:

Dim concatSheet As Worksheet
Set concatSheet = ThisWorkbook.Sheets("CONCAT")
CODART = concatSheet.Range("A2").Value

Use With Statements: Utilize the With statement to work with ranges efficiently without repeating the sheet reference.

Replace:

ActiveCell.Offset(0, 2).Select 
CODART = ActiveCell.Value
ActiveCell.Offset(0, 1).Select 
CODCLR = ActiveCell.Value
ActiveCell.Offset(0, 1).Select 
TALLE = ActiveCell.Value

with:

With ActiveCell
 CODART = .Value
 CODCLR = .Offset(0, 1).Value
 TALLE = .Offset(0, 2).Value
End With

Minimize the Use of Loops: Avoid using loops when not necessary. In your case, you can directly work with ranges and cells without the need for a loop.

Replace:

While ActiveCell.Value <> "" And ActiveCell.Value <> Val(L) And ActiveCell.Value <> L
 ActiveCell.Offset(1, 0).Select
Wend

with:

On Error Resume Next
Set foundCell = concatSheet.Columns(1).Find(Val(L), LookIn:=xlValues)
On Error GoTo 0
If Not foundCell Is Nothing Then
 ' Your logic here
End If

These suggestions aim to reduce the reliance on selecting cells and looping through them, which should improve the performance of your code. Adjustments may be needed based on your specific requirements.

Tips:

• Always Declare variables with their respective types,
• Avoid using ActiveCell as it heads overhead to code
• Avoid using .Select multiple times specifically in Loops
• Refer Sheets/Ranges with fully qualifiable reference and use this variable in the code instead of referring the worksheet name every time.

I hope that helps!

• Vipul Karkar: I came back to code review after a very long time and didn't notice that I was on code review instead of StackOverflow. Added the code review instead of new code itself.
• Billal BEGUERADJ: Welcome back. Now it looks good. +1 :)