Excel Vba; Any idea of how to improve this code? (simple loop but ...)

Question

I have a huge macro and it Works fine except for this part of the code I do not know why it takes so much time (other calculations are similar or even bigger)

b -> is number of rows of this sheet

a -> is the number of times it has to move (this number comes from another sheet)

I am sure that there are multiple ways to improve this. (I already have screeupdating and calculation at manual)

 For x = 1 To b
 Range("Z1").Select
 ActiveCell.Offset(x, 0).Select
 For i = 1 To (a - 1)

 If ActiveCell.Value <> 0 Then
    d = Sheets("AGREGADO").Range("a1048576").End(xlUp).Row
    Sheets("AGREGADO").Cells(d + 1, 1).Value = Sheets("TODO").Cells(x + 1, 7).Value
    Sheets("AGREGADO").Cells(d + 1, 3).Value = Sheets("TODO").Cells(x + 1, 25 + i).Value
    Sheets("AGREGADO").Cells(d + 1, 2).Value = Sheets("TODO").Cells(1, 25 + i).Value
    Sheets("AGREGADO").Cells(d + 1, 4).Value = Sheets("TODO").Cells(x + 1, 32 + a).Value
End If

    ActiveCell.Offset(0, 1).Select
Next i
Next x

Thanks in advance!

Remove .Select and use of ActiveCell and instead use Cells(row,column) — QHarr, Jun 07 '18 at 09:50
https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba — Vityata, Jun 07 '18 at 09:50
Working code in need of optimization might be better on [CodeReview](https://codereview.stackexchange.com/) — Vincent G, Jun 07 '18 at 09:53
And fully qualify ranges with sheet names. Are you looping column Z of TODO for example? — QHarr, Jun 07 '18 at 09:53
@VincentG I agree though the code above is bug prone to say the least. — QHarr, Jun 07 '18 at 09:53
Checking `Sheets("AGREGADO").Range("a1048576").End(xlUp).Row` each iteration of the loop will slow things down too. Check it at the beginning and keep track of the output row, incrementing it as necessary instead of scanning the entire sheet each time. — CLR, Jun 07 '18 at 09:55
Also interaction with each cell isn't the best. You can load the data into arrays and interact with the arrays. — Aneta, Jun 07 '18 at 10:18
Are you remembering to turn of AutoCalculation and ScreenUpdating before you run the loop? — Chronocidal, Jun 07 '18 at 10:23
I'm voting to close this question as off-topic because questions asking to improve code are off-topic at Stack Overflow. If you have a specific problem with your code, please specify that and ask how the problem can be fixed. Otherwise, consider asking on Code Review if your question meets their requirements (no pseudo-code, you can share the *whole* codeset, etc.). — TylerH, Jun 07 '18 at 14:46

CLR · Accepted Answer · 2018-06-07T10:41:10.677

I've had a go.. but I may have made too many assumptions or missed something vital but without seeing the source data it's difficult.

Try something like this:

With Sheets("AGREGADO")
    d = .Range("a1048576").End(xlUp).Row
    For x = 1 To b
        For i = 1 To (a - 1)
            If Sheets("TODO").Cells(x + i + 1, 26).Value <> 0 Then
                .Cells(d + 1, 1).Value = Sheets("TODO").Cells(x + 1, 7).Value
                .Cells(d + 1, 3).Value = Sheets("TODO").Cells(x + 1, 25 + i).Value
                .Cells(d + 1, 2).Value = Sheets("TODO").Cells(1, 25 + i).Value
                .Cells(d + 1, 4).Value = Sheets("TODO").Cells(x + 1, 32 + a).Value
                d = d + 1
            End If
        Next i
    Next x
End With

Excel Vba; Any idea of how to improve this code? (simple loop but ...)

1 Answers1