CODEWITHSUNDEEP
excelexcel-2007vba
Pramod
Pramod

Reputation: 1471

Excel not responding after running macro

I used the below code to copy a column from one sheet to another and then replace the blank cells with a Null value:

'Copying If Employee
sourceSheet.Activate
Range(Cells(2, 7), Cells(Rows.Count, 7).End(xlUp)).Select
Selection.Copy
destSheet.Activate
Range("E2", Cells(Rows.Count, 7)).PasteSpecial

For Each cell In Range("E2", Cells(Rows.Count, 5))
If Len(cell.Value) = 0 Then
    cell.Value = "No"
End If

When I replace the for statement with Range("E2", Cells(500,5)) it is working fine.

What might be the problem? I was unable to figure out. Can anyone help me with this?

Upvotes: 0

Views: 4015

Answers (3)

CaBieberach
CaBieberach

Reputation: 1768

Excel 2007 has 1,048,576 rows.

  1. Your For each loop has to go through a million cells and each time it has to load a cell (with all its properties) into the memory. This constant loading and unloading of cell objects is very resource consuming.

  2. Each time you assign the value No to a cell, Excel recalculates the sheet, refreshes the screen and might trigger worksheets/workbook/cell events. All this before evaluating the next cell to the next cell. Unless you have a HUGE amount of data, this will be happening a couple hundred tousand times in this example.

500 cells is a much smaller number and can be handled without problem.

Edit:

Geting the last row.

If you want to get the last visible row with data, then use:

With Workbooks(myWorkbook).Worksheets(myWorksheet)
   LastRow = .Cells(.Rows.Count, "E").End(xlUp).Row
End With

If you want to get the last row with data, regardless of it being visible or hidden, then use:

With Workbooks(myWorkbook).Worksheets(myWorksheet)
   LastRow =.Range("E2").EntireColumn.Find("*", .Cells(1, .Range("E2").Column), , , xlByRows, xlPrevious).Row
End With

For more info Rondebruin has a good site with more info about geting the las row/column.

Upvotes: 8

Peter Albert
Peter Albert

Reputation: 17495

As Cabierberach rightly points out, your current routine loops over 1M lines. In addition to his and JustinJDavies solution, consider this hack:

Instead of setting the values 0 to No, you simply can change the format if the cells to display No every time the value is 0. This can be done without a macro by applying this custom format to the cells: 0,-0,"No",@. You might need to replace the 0with your default format - see this link for a detailed description of the custom number format.

If you want to do this in VBA, this line will suffice:

Range("E2:E" & Cells(Rows.Count, 1).End(xlUp).Row).CustomFormat = "0,0,""No"",@"

Upvotes: 2

JustinJDavies
JustinJDavies

Reputation: 2693

Try 

For Each cell In Range("E2", Cells(destSheet.UsedRange.Rows.Count, 5))
If Len(cell.Value) = 0 Then
    cell.Value = "No"
End If

Or more elegantly (and probably a lot faster),

Dim calcStatus As XlCalculation
calcStatus = Application.Calculation

Application.ScreenUpdating = False
Application.Calculation = xlCalculationManual

For Each cell In Range("E2:E" & destSheet.UsedRange.Rows.Count)
If Len(cell.Value) = 0 Then
    cell.Value = "No"
End If    

Application.Calculation = calcStatus
Application.ScreenUpdating = True

Upvotes: 2

Related Questions