Return to Answer

Added code review rather than sharing the entire optimized code.

Source Link

edited Dec 26, 2023 at 5:43

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:

Sub LSTART_DblClickSheets(ByVal Cancel As MSForms.ReturnBoolean"CONCAT")
    On Error GoTo ERR1
    
    Application.ScreenUpdating = False
Select
    Dim selectedValue As VariantRange("A2").Select

with:

Dim concatSheet As Worksheet
    Dim ventasSheet As Worksheet

    selectedValue = LSTART.List(LSTART.ListIndex, 0)
    Set concatSheet = ThisWorkbook.Sheets("CONCAT")
    Set ventasSheetCODART = ThisWorkbookconcatSheet.SheetsRange("ventas""A2")

 .Value
    Dim concatRange

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

Replace:

ActiveCell.Offset(0, As2).Select Range
    Set concatRangeCODART = concatSheetActiveCell.Columns(1)Value

ActiveCell.FindOffset(What:=selectedValue, LookIn:=xlValues0, LookAt:=xlWhole1)

    If Not concatRange.Select Is Nothing Then
        ' Retrieve dataCODCLR from= CONCATActiveCell.Value
        Dim CODART As Variant
    ActiveCell.Offset(0, 1).Select   Dim CODCLR As Variant
        Dim TALLE As= Variant
ActiveCell.Value

with:

With ActiveCell
    CODART = concatRange.Offset(0, 2).Value
        CODCLR = concatRange.Offset(0, 1).Value
        TALLE = concatRange.Offset(0, 12).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 ventasSheetActiveCell.Range("B"Value &<> ventasSheet.Rows.Count).EndVal(xlUpL).Offset(1, 0)And ActiveCell.Value =<> CODARTL
        ventasSheet.Range("B" & ventasSheet.Rows.Count).End(xlUp)ActiveCell.Offset(01, 10).Value = CODCLRSelect
  Wend

with:

On Error Resume Next
Set foundCell = ventasSheetconcatSheet.RangeColumns("B" & ventasSheet.Rows.Count1).EndFind(xlUp).OffsetVal(0L), 2LookIn:=xlValues).Value
On =Error TALLEGoTo 0

If Not foundCell Is EndNothing If

ERR1:Then
    Application.ScreenUpdating' =Your Truelogic here
End SubIf

FewThese 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:

Sub LSTART_DblClick(ByVal Cancel As MSForms.ReturnBoolean)
    On Error GoTo ERR1
    
    Application.ScreenUpdating = False

    Dim selectedValue As Variant
    Dim concatSheet As Worksheet
    Dim ventasSheet As Worksheet

    selectedValue = LSTART.List(LSTART.ListIndex, 0)
    Set concatSheet = ThisWorkbook.Sheets("CONCAT")
    Set ventasSheet = ThisWorkbook.Sheets("ventas")

 
    Dim concatRange As Range
    Set concatRange = concatSheet.Columns(1).Find(What:=selectedValue, LookIn:=xlValues, LookAt:=xlWhole)

    If Not concatRange Is Nothing Then
        ' Retrieve data from CONCAT
        Dim CODART As Variant
        Dim CODCLR As Variant
        Dim TALLE As Variant

        CODART = concatRange.Offset(0, 2).Value
        CODCLR = concatRange.Offset(0, 1).Value
        TALLE = concatRange.Offset(0, 1).Value
 
        ventasSheet.Range("B" & ventasSheet.Rows.Count).End(xlUp).Offset(1, 0).Value = CODART
        ventasSheet.Range("B" & ventasSheet.Rows.Count).End(xlUp).Offset(0, 1).Value = CODCLR
        ventasSheet.Range("B" & ventasSheet.Rows.Count).End(xlUp).Offset(0, 2).Value = TALLE
    End If

ERR1:
    Application.ScreenUpdating = True
End Sub

Few Tips:

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:

Source Link

created Dec 22, 2023 at 8:57

Vipul Karkar

Sub LSTART_DblClick(ByVal Cancel As MSForms.ReturnBoolean)
    On Error GoTo ERR1
    
    Application.ScreenUpdating = False

    Dim selectedValue As Variant
    Dim concatSheet As Worksheet
    Dim ventasSheet As Worksheet

    selectedValue = LSTART.List(LSTART.ListIndex, 0)
    Set concatSheet = ThisWorkbook.Sheets("CONCAT")
    Set ventasSheet = ThisWorkbook.Sheets("ventas")


    Dim concatRange As Range
    Set concatRange = concatSheet.Columns(1).Find(What:=selectedValue, LookIn:=xlValues, LookAt:=xlWhole)

    If Not concatRange Is Nothing Then
        ' Retrieve data from CONCAT
        Dim CODART As Variant
        Dim CODCLR As Variant
        Dim TALLE As Variant

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

        ventasSheet.Range("B" & ventasSheet.Rows.Count).End(xlUp).Offset(1, 0).Value = CODART
        ventasSheet.Range("B" & ventasSheet.Rows.Count).End(xlUp).Offset(0, 1).Value = CODCLR
        ventasSheet.Range("B" & ventasSheet.Rows.Count).End(xlUp).Offset(0, 2).Value = TALLE
    End If

ERR1:
    Application.ScreenUpdating = True
End Sub

Few 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!