PDA

View Full Version : Solved: Could this be a buffering problem?



CodeNinja
04-15-2013, 06:54 AM
I wrote some very simple code (or so I thought) that performs an advanced filter for a co-worker and compares the results of that advanced filter. The code runs beautifully on my machine and would consistently break on my co-workers. We both have excel 2010 windows 7.

On his computer, we get the little circle that excel is thinking and then excel is not responding. It doesn't actually break from runtime, excel crashes.

I was able to locate where the program crashes... It happens inside a loop:

For lRowAF = 2 To wsST.Range(getColLtr(iModule2) & "65536").End(xlUp).Row
If wsST.Cells(lRowST, 1) & wsST.Cells(lRowST, 2) & wsST.Cells(lRowST, 3) <> wsST.Cells(lRowAF, iModule2) & wsST.Cells(lRowAF, iModule2 + 1) & wsST.Cells(lRowAF, iModule2 + 2) Then
bMatchFound = True
lRowAF = wsST.Range(getColLtr(iModule2) & "65536").End(xlUp).Row
End If
Next lRowAF

What really gets my goat is that when I changed that code to:

For lRowAF = 2 To wsST.Range(getColLtr(iModule2) & "65536").End(xlUp).Row
str1 = wsST.Cells(lRowST, 1) & wsST.Cells(lRowST, 2) & wsST.Cells(lRowST, 3)
str2 = wsST.Cells(lRowAF, iModule2) & wsST.Cells(lRowAF, iModule2 + 1) & wsST.Cells(lRowAF, iModule2 + 2)
If str1 <> str2 Then
bMatchFound = True
lRowAF = wsST.Range(getColLtr(iModule2) & "65536").End(xlUp).Row
End If
Next lRowAF

... The program works on his computer most of the time. It will still crash sometimes.

Obviously those two things are virtually the same. I am just comparing 2 strings versus comparing the concatenation. My question is two fold:

1- Is this a buffering problem? If not, what is going on to cause this to crash?

2- What is happening "under the hood" of excel that is different between the two above code snippets to cause one to crash and one to work?


Thanks ever so much,

CodeNinja.

snb
04-15-2013, 07:17 AM
Are you familair with 'Exit for' ?

Sub M_snb()
sn= wsST.columns(imodule2).specialcells(2).resize(,3)
For j=2 to ubound(sn)
If sn(j,1) & sn(j,2) & sn(j,3) <> wsST.Cells(lRowAF, iModule2) & wsST.Cells(lRowAF, iModule2 + 1) & wsST.Cells(lRowAF,iModule2 + 2) Then exit for
Next
notidentical=j<>ubound(sn)+1
End Sub

CodeNinja
04-15-2013, 07:54 AM
snb,
Thank you very much for your response.
I think (heh, I KNOW) you are a little beyond me on this one... I am testing to see if the match is found. If it is found, I set boolean to true and basically exit the for by setting the loop counter to the end of the loop. As I see it, an "exit for" might allow me to save 2 procedures.
To me, your code looks like you are leaving J (the loop counter) at where the match is and exiting the for... and if the match is not equal to the maximum (ubound) of the count of rows, you assume it is not equal.
I have three questions about this code. First, what happens if the match is the very last row? Wouldn't the code say "they are equal to the ubound therefore there is no match"? Second, how is this any more efficient than my code? Finally, I still have no idea why my code was crashing his excel... any clues?

Thanks so much,

CodeNinja.

snb
04-15-2013, 09:12 AM
Apparently you want to compare two 3-columns of data ?
Are those columns in the same worksheet or not ?
I'd prefer to put the values in array variables (1 if the columns are in the same worksheet, in 2 if they are in separate worksheets). The comparison will run much faster that way.

if you use a loop:

for j=1 to 10

next

the value of j, after fully cycling the loop will be 11. (That's the reason the loop stops).
If the loop will be ended earlier the value will be 10 or less.
So testing whether the value of j is 1 higher than the number of preset loops indicates whether or not any condition to exit the loop was true.

Guessing about your problem: changing the value of a loop parameter might interfere with the loop's dynamic (changing the loop parameter itself).
Stepping through the code ( F8 ) might shed some more light on this.

Paul_Hossler
04-15-2013, 09:25 AM
some thoughts and a little untested code


'2007 and 2010 have 1M+ rows, more than 65536 of older versions
'if there are more than 65536 rows and since you start there, .End will return row 1
'if there are spaces in cell, .End will find those
'Why are you resetting the loop variable ....?? lRowAF = .Range( .....
Sub Piece()
With wsST
For lRowAF = 2 To .Range(getColLtr(iModule2) & .Rows.Count).End(xlUp).Row
If .Cells(lRowST, 1) & .Cells(lRowST, 2) & .Cells(lRowST, 3) <> _
.Cells(lRowAF, iModule2) & .Cells(lRowAF, iModule2 + 1) & .Cells(lRowAF, iModule2 + 2) Then
bMatchFound = True
lRowAF = .Range(getColLtr(iModule2) & .Rows.Count).End(xlUp).Row
End If
Next lRowAF
End With
End Sub


How are you trying to compare, and can you post a small WB?

Paul

SamT
04-15-2013, 09:33 AM
Not tested. You might want to convert iModule2 to a long. :dunno

'Troubleshooting: Use in situation where XL crashes
MsgBox ("String to Compare is " & _
str1 = wsST.Cells(lRowST, 1) & wsST.Cells(lRowST, 2) & wsST.Cells(lRowST, 3) _
& Chr(13) & "Str2 Column is " & getColLtr(iModule2))

For lRowAF = 2 To wsST.Range(getColLtr(iModule2) & wsST.Rows).End(xlUp).Row
str1 = wsST.Cells(lRowST, 1) & wsST.Cells(lRowST, 2) & wsST.Cells(lRowST, 3)
str2 = wsST.Cells(lRowAF, iModule2) & wsST.Cells(lRowAF, iModule2 + 1) & wsST.Cells(lRowAF, iModule2 + 2)

'Troubleshooting, replace 1000 with 500 or as needed
If (lRowAF Mod 1000 = 0) Then
MsgBox ("AF Row is " & lRowAF _
& Chr(13) & "ST Row is " & lRowST)
ShowProcesses 'Make sure you have a "Sheet3" in the Workbook
End If

If str1 <> str2 Then 'str1 and str2 are not the same
bMatchFound = True
ExitLoop
End If
Next lRowAF


Sub ShowProcesses()
'Written: July 06, 2011
'Author: Leith Ross

Dim arrData As Variant
Dim arrLabels As Variant
Dim N As Long
Dim Rng As Range
Dim RowCnt As Long
Dim sngProcessTime As Single
Dim Wks As Worksheet

'Worksheet and starting cell of Process List
Set Wks = Worksheets("Sheet3")
Set Rng = Wks.Range("A1")

'Local computer
strComputer = "."

Set objWMIService = GetObject("winmgmts:" _
& "{impersonationLevel=impersonate}!\\" _
& strComputer & "\root\cimv2")

Set colProcesses = objWMIService.ExecQuery _
("Select * from Win32_Process")


arrLabels = Array("Process:", "Processor Time:", "Process ID:", _
"Working Set Size:", "Page File Size:", "Page Faults:")

RowCnt = UBound(arrLabels) + 1


For Each objProcess In colProcesses

sngProcessTime = (CSng(objProcess.KernelModeTime) + _
CSng(objProcess.UserModeTime)) / 10000000

With objProcess
arrData = Array(.Name, sngProcessTime, .ProcessId, _
.Workingsetsize, .PageFileUsage, .PageFaults)
End With

With Rng.Resize(RowCnt, 1)
.Offset(N, 0) = WorksheetFunction.Transpose(arrLabels)
.Offset(N, 1) = WorksheetFunction.Transpose(arrData)
End With

N = N + UBound(arrData) + 2

Next objProcess

End Sub

CodeNinja
04-15-2013, 01:41 PM
Wow, Thanks everyone for the responses. I'm going to do my best to answer all questions.

SNB:

Apparently you want to compare two 3-columns of data ?
Are those columns in the same worksheet or not ?
Yes, I am using 3 columns of data.
The two compared ranges are in the same worksheet (wsSt). One list may be longer than the other.
the value of j, after fully cycling the loop will be 11. (That's the reason the loop stops).
If the loop will be ended earlier the value will be 10 or less.
So testing whether the value of j is 1 higher than the number of preset loops indicates whether or not any condition to exit the loop was true.


Guessing about your problem: changing the value of a loop parameter might interfere with the loop's dynamic (changing the loop parameter itself).
Stepping through the code ( F8 ) might shed some more light on this.

Thanks, the exit for makes more sense now... I will try this to see if I get better performance.

I was able to identify this portion of code as where the crash occurred by stepping through and using breaks, however, the data is around 3-500 rows per list, and I do not feel like stepping through that individually.


Paul:

'2007 and 2010 have 1M+ rows, more than 65536 of older versions
'if there are more than 65536 rows and since you start there, .End will return row 1
'if there are spaces in cell, .End will find those
You are absolutely right... 65536 is a 2003 thing, I guess old habits die hard. Thanks for pointing this out. I don't think it really makes much difference as our use will never get past maybe 5-6k records, but you are 100% correct.

How are you trying to compare, and can you post a small WB?
Just trying at that point to find rows that do not have a match. I will post a sample workbook separately in a few minutes.

Sam:

You might want to convert iModule2 to a long.
Not sure what that gets me sam. iModule is the column of the second module of data. It's probably something like 11. It doesn't vary much. If it did, I might agree.

I am not sure what your second snippet of code does... would you please explain its purpose?

Thanks so much everyone.

CodeNinja
04-15-2013, 01:48 PM
Per your request I am uploading sample data... 3 files this is file 1. Not sure how to upload more in 1 post, so I apologize about 3 posts...

CodeNinja
04-15-2013, 01:49 PM
Second file

CodeNinja
04-15-2013, 01:50 PM
This is the control program that compares the two

snb
04-15-2013, 03:32 PM
to filter the non matching elements in columns A & B & C:


Sub M_snb()
sn = Workbooks("__bestand_001.xlsx").Sheets(1).Cells(1).CurrentRegion.Resize(, 3)
sp = Workbooks("__bestand_002.xlsx").Sheets(1).Cells(1).CurrentRegion.Resize(, 3)

For j = 2 To UBound(sn)
c00 = c00 & vbCr & Join(Application.Index(sn, j), "|")
Next

For j = 2 To UBound(sp)
c01 = c01 & vbCr & Join(Application.Index(sp, j), "|")
Next

sn = Split(Mid(c00, 2), vbCr)
For j = 1 To UBound(sn)
If InStr(c01 & vbCr, vbCr & sn(j) & vbCr) Then sn(j) = "~"
Next

MsgBox Join(Filter(sn, "~", False), vbLf)
End Sub

SamT
04-15-2013, 03:45 PM
I am not sure what your second snippet of code does... would you please explain its purpose?

Thanks so much everyone.

This bit in the top code calls Sub Showprocesses() at each 1000 (or whatever) loops.
If you are not looping a lot, you should replace 1000 with 100 or even less. You want to see enough rows on sheet3 to notice any usages that might be climbing with the loop count.

Calls ShowProcesses: 'Make sure you have a "Sheet3" in the active Workbook
'Troubleshooting, replace 1000 with 500 or as needed
If (lRowAF Mod 1000 = 0) Then
MsgBox ("AF Row is " & lRowAF _
& Chr(13) & "ST Row is " & lRowST)
ShowProcesses
End If

ShowProcesses records a bunch-a-ton-a information, including memory usage, on Sheet3.

Paul_Hossler
04-15-2013, 04:46 PM
Minor suggestions


1. In case the user cancels, you can exit without runtime error


Dim vName As Variant
v = GetFileName
If v <> False Then
Set wbST = Workbooks.Open(Filename:=v)
Else
Exit Sub
End If


and

2. I changed your filefilter so that it would show both old and new files
GetFileName = Application.GetOpenFilename(filefilter:="Excel files (*.xls; *.xlsx), *.xls; *.xlsx", Title:=msg)

3. I don't use col letters, but Cells w/o an explicit reference will refer to whatever the activesheet happens to be. I've found that to generate erratic results. If you want to use column letters, you might pass the worksheet


Private Function getColLtr(ws As Worksheet, ByVal i As Integer) As String
'returns a column letter from a given number (ie: 1 returns "A", 2 returns "B", 27 returns "AA")
getColLtr = Split(ws.Cells(1, i).Address, "$")(1)
End Function


Paul

CodeNinja
04-16-2013, 07:17 AM
Wow Wow Wow!!!! such great information from all of you! Thanks so much guys.

SNB, I will play with this and see if I can get it to work for me. Thanks.

Sam, Great tool, I will play with this also. Seems like a great diagnostic tool.

Paul, I have been meaning to add some error handling (I try to get things working first typically). I also have been meaning to fix that file filter, so thanks a bunch. Lastly, I have never had a problem with getcolltr, but I don't see how adding the workbook reference could hurt, and if it avoids headaches in the future, I'll do it, so thanks again.

With all this great help, I'm sure I'll get this to work on my co-worker's computer properly. For some reason, it still bothers me that it didn't work the first time. *Shrug* I guess computers are fickle things. Part of the fun I guess.

Thanks again for everything everyone. I will play with this a little more and if the suggestions work I will mark the thread solved. If not, I'm sure you will all hear from me again. :beerchug:

snb
04-16-2013, 09:41 AM
if you put this macro in the second workbook maybe this is sufficient:

Sub tst()
c00 = LCase(Join([transpose('[__bestand_001.xlsx]worksheet'!A1:A40&" "&'[__bestand_001.xlsx]worksheet'!B1:B40&" "&'[__bestand_001.xlsx]worksheet'!C1:C40)], vbLf))
sn = [transpose(A1:A57&" "&B1:B57&" "&C1:C57)]

For j = 1 To UBound(sn)
If InStr(vbLf & c00 & vbLf, vbLf & LCase(sn(j)) & vbLf) Then sn(j) = "~"
Next

MsgBox Join(Filter(sn, "~", False), vbLf)
End Sub

NB. the code tags interpreter in this forum misinterprets the ' as comment indicator.