Can someone help me with this slow code

B

BillReese

Hi,
I have a VB application, which starts and controls Excel. The program reads
the data from over 20 ASCII files and extracts that data and stitches it
together into one workbook by copy and paste actions.

Here is my problem, I do a lot of other things with my data, that goes very
quickly... I mostly set ranges to do this and either move or modify data
as ranges, or worksheet formulas. This code I past below is one section
where I don't know how to work with it as a range.

My problem code iterates a column to and find any occurrence of the value
"0".. If it finds a Zero it's a bad reading recorded by this instrument (a
spectrometer).. then I need to replace it with the previous row's value
because an electrical dropout is not a real reading, and a ZERO give me
divide by Zero errors for my other formulas added to the main spreadsheet...

does anyone know of a faster way to do this code below. If I write this
code inside an Excel file as a local macro, it runs extremely swift, but
when I use Excel as a remote it takes maybe 20 to 50 seconds this relatively
small iteration.

can someone please help me on this?
=====================================================================================
Dim Ws As Worksheet, x As Long, y As Long
Set Ws = ActiveSheet: y = 2
With Ws
Application.DisplayAlerts = False
For x = 1 To 2000
If (Cells((500 + x), y).Value = "0") Then
Cells((500 + x), y).Value = Cells((500 + (x - 1)), y).Value
End If
Next
End With
Application.DisplayAlerts = True
=====================================================================================
 
B

Bob Phillips

Why not just change the formulae to ignore 0

IF(rng<>0,rng)

within the formula, making it an array formula


--

HTH

RP
(remove nothere from the email address if mailing direct)
 
B

BillReese

A zero reading from the spectrometer means that the refresh rate was not
good enough to record a value.. I taking maybe 6 to 20 columns and dividing
it by the first 10 columns... The first ten columns many times have one,
two or 3 zeros in them at certain wavelengths... but there are times when
all the columns are reading 0 at the same wavelength).. So if I get rid of
the zeros then I'd still have an average of a number of columns divided by
"nothing" (or empty cells).. so that will not solve my problem...

The truth is that there is no possible way to have a zero there, it's a
machine problem.. and I am stuck with the machine.. it has to do with an
electrical impulse that does not dump it's voltage quick enough on a refresh
and this is what causes the problem. The correct value for that point is
extremely close to the previous value in the row before it.. I just need to
refer each of those cells to the previous cell..

I can do what I need to do with the code I sent before.. But the code is
horribly slow.. I think there must be a better way, but I don't know how so
I am looking for an expert to point me in the right direction.

thanks,
BillReese
 
B

Bob Phillips

My biggest problem is that I couldn't get it to take a long time, 1/8th
second was all it took.

I could improve it with this

Set Ws = ActiveSheet: y =2
With Ws
Application.DisplayAlerts = False
For x = 501 To 2500
If (Cells(x, y).Value = 0) Then
Cells(x, y).Value = Cells((x - 1), y).Value
End If
Next
End With
Application.DisplayAlerts = True

but something else must be happening.

Do you have worksheet events? If so, turn them off beforehand, and
screenupdating and calculation

Application.EnableEvents = False
Application.Calculation = xlCalculation Manual
Application.ScreenUpdating = False

Set Ws = ActiveSheet
y = 4
With Ws
Application.DisplayAlerts = False
For x = 501 To 2500
If (Cells(x, y).Value = 0) Then
Cells(x, y).Value = Cells((x - 1), y).Value
End If
Next
End With
Application.DisplayAlerts = True

Application.ScreenUpdating = True
Application.Calculation = xlCalculation Automatic
Application.EnableEvents = True


--

HTH

RP
(remove nothere from the email address if mailing direct)
 
B

Bob Phillips

George,

1) is a good point.

2) doesn't sound so good to me as it might just slow it if the default
property has to be looked up to see what it is.

--

HTH

RP
(remove nothere from the email address if mailing direct)
 
G

George Nicholson

2 possible additions to your suggestions:

1) The code uses a "With.. With End" structure but nothing is relying on
that. Changing Cells() to .Cells() might actually improve things, especially
if this is running in VB not Excel. VB might be spending time resolving each
ambiguous Cells() occurance. Clarification might help performance.

2) The flip side of that: Value is the default property of Cells, so it
could probably be omitted.

HTH,
 
B

BillReese

Bob Phillips said:
My biggest problem is that I couldn't get it to take a long time, 1/8th
second was all it took.


Bob, perhaps you have a fast 'puter and it goes that quickly. On my puter
it is quite slow.. I am using my 'puter at work, it's around 7 years old or
so.. I am sure my home puter (3 Ghz) would be much faster. It probably
takes around 20 seconds or so on each file at work.. after doing that for
16 files or so it's painful slow. As I said before if I put the code in a
local file it was blazing fast, my problem was only when using Excel as an
automation object from inside a VB app. I also was using it as a bound
object using the "new" operator, not by using "CreateObject()" to gain
access to Excel.

I actually found my own remedy.. Your remedy really did not change
anything as far as the speed goes, if it did the change was very tiny. In
fact when I took one line I was uisng to replace the text if the condition
was met...and also turning everthing off (calculations, and updating) like
you suggested.. I was then only reading a value out of a cell and
comparing it to a hard coded string value.. But the routine still took
almost the same time.

The remedy I decided on:
I remembered this problem is highlighted in my excel 97 developers'
handbook (Wells, Harshbanger).. The trick is to assign a range to an
array, then compare and alter the array.. then finally reassign the array to
the range. End result is that it's so fast I don't even notice a pause
happening between routines. Guess I could have made my code slightly more
efficient if I "Set" the range at the start of my code below.. but it
works just fine now.. it's plenty fast enough, so I don't care to worry
about that.

thanks for your comments.
'########################################################################################################
Dim RG_Array As Variant
call xlPause( ExclObj, True )
Set WS = WBmain.Worksheets("Plots")
With WS
RG_Array = .Range(Cells(500, 2), Cells(2502,
17)).Value
For cnt = 2 To UBound(RG_Array, 1) - 1
For Y = 1 To UBound(RG_Array, 2)
If (RG_Array(cnt, Y) = "0") Then
RG_Array(cnt, Y) = RG_Array((cnt -
1), Y)
End If
Next
Next
.Range(Cells(500, 2), Cells(2502, 17)).Value =
RG_Array
End With
call xlPause( ExclObj, False )
'########################################################################################################
sub xlPause( xl as object, bVal as boolean)
bVal = Not bVal
xl.ScreenUpdating = bVal
xl.EnableEvents = bVal
if bVal = true then
xl.Calculation = xlCalculationAutomatic
else
xl.Calculation = xlCalculationManual
end if
end sub
'########################################################################################################
 
B

Bob Phillips

Hi Bill,

Interesting, thanks for sharing it with us.

I will have to try that out on my 97 machine as soon as I dust it off :)

Bob
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top