PDA

View Full Version : Solved: The Trouble with For-Loops



Saladsamurai
08-27-2009, 12:16 PM
is that I suck at them.

I have attached file.

I have done a CountIf worksheet function to see how many cells contain data in WorkSheets("Best CI CFD"). It is about 384.

Now, in my code, in the first block (the only part UN commented-out) I have a nested For Loop / Nested If statements that check to see whether
WorkSheets("Best CI CFD").Cells(i, j) meets certain criteria.

If WorkSheets("Best CI CFD").Cells(i, j) >= .9 Then G = G+1

If WorkSheets("Best CI CFD").Cells(i, j) from .8 to .9 Then Y = Y+1

If WorkSheets("Best CI CFD").Cells(i, j) < .8 Then R = R+1

When I MsgBox R and Y, the results seem reasonable, though I cannot be sure. At least they are both < 384

However, when I MsgBox G, I get something like 465, which is greater then the number of cells that contain data!

Here is the code. Like I said, I commented out the superfluous stuff (most of it at least):

Option Explicit
Private Sub CommandButton1_Click()

Dim i, j, nRow, nCol, G, Y, R, TotalRacks As Long
Dim xgg, xgy, xgr, xyg, xyy, xyr, xrg, xry, xrr As Long
Dim OverPredicted, MyError, Percent, WithInTenPercent As Double
Dim MyMax, ErrorSquare As Double


' *********************************************************************
' *********************************************************************
' Count # of times ISX is conservative or not


nRow = 25
nCol = 32

xgg = 0
xgy = 0
xgr = 0
xyg = 0
xyy = 0
xyr = 0
xrg = 0
xry = 0
xrr = 0
G = 0
Y = 0
R = 0

MyMax = 0
Percent = 0
OverPredicted = 0
WithInTenPercent = 0
ErrorSquare = 0
' Check # times CFD predicts Green and ISX predicts Green, Yellow, or Red
For i = 1 To nRow
For j = 1 To nCol


If Worksheets("Best CI CFD").Cells(i, j) >= 0.9 Then
If Worksheets("Best CI ISX").Cells(i, j) >= 0.9 Then
xgg = xgg + 1
ElseIf Worksheets("Best CI ISX").Cells(i, j) < 0.9 And Worksheets("Best CI ISX").Cells(i, j) > 0.8 Then
xgy = xgy + 1
ElseIf Worksheets("Best CI ISX").Cells(i, j) <= 0.8 And Worksheets("Best CI ISX").Cells(i, j) <> "" Then
xgr = xgr + 1
End If

G = G + 1

' ElseIf Worksheets("Best CI CFD").Cells(i, j) < 9 And Worksheets("Best CI CFD").Cells(i, j) > 0.8 Then
' If Worksheets("Best CI ISX").Cells(i, j) >= 0.9 Then
' xyg = xyg + 1
' ElseIf Worksheets("Best CI ISX").Cells(i, j) < 0.9 And Worksheets("Best CI ISX").Cells(i, j) > 0.8 Then
' xyy = xyy + 1
' ElseIf Worksheets("Best CI ISX").Cells(i, j) <= 0.8 And Worksheets("Best CI ISX").Cells(i, j) <> "" Then
' xyr = xyr + 1
' End If
'
' Y = Y + 1
'
' ElseIf Worksheets("Best CI CFD").Cells(i, j) <= 0.8 And Worksheets("Best CI CFD").Cells(i, j) <> "" Then
' If Worksheets("Best CI ISX").Cells(i, j) >= 0.9 Then
' xrg = xrg + 1
' ElseIf Worksheets("Best CI ISX").Cells(i, j) < 0.9 And Worksheets("Best CI ISX").Cells(i, j) > 0.8 Then
' xry = xry + 1
' ElseIf Worksheets("Best CI ISX").Cells(i, j) <= 0.8 And Worksheets("Best CI ISX").Cells(i, j) <> "" Then
' xrr = xrr + 1
' End If
'
' R = R + 1
'
End If
Next j
Next i
'' *********************************************************************
'' *********************************************************************
'' Count # times ISX Overpredicted; Is w/in 10 %; max error; etc
'
' For i = 1 To nRow
' For j = 1 To nCol
' If Worksheets("Best CI CFD").Cells(i, j) <> "" Then
'' Compute Difference: ISX_CI -CFD_CI
' MyError = Worksheets("Best CI ISX").Cells(i, j) - Worksheets("Best CI CFD").Cells(i, j)
'
'
' If MyError > 0 Then
' OverPredicted = OverPredicted + 1 'Overpredicted means ISX predicted better cooling
' End If 'performance than actual (i.e. CFD)
'
'' Compute % Error by dividing by actual (i.e. CFD)
' Percent = Percent + Abs(MyError) / (Worksheets("Best CI CFD").Cells(i, j) + 0.0000000001) 'Prevents a div by 0 error
'
' If Abs(MyError) > MyMax Then
' MyMax = Abs(MyError)
' End If
'' Count #times ISX is w/in 10 % of CFD
' If Abs(MyError) <= 0.1 Then
' WithInTenPercent = WithInTenPercent + 1
' End If
'
'' Calculate error-squared-sum for RMS calculation
' ErrorSquare = ErrorSquare + MyError ^ 2
' End If
' Next j
' Next i
'
'
' For i = 1 To nRow
' For j = 1 To nCol
' If Worksheets("Best CI CFD").Cells(i, j) <> "" Then
' TotalRacks = TotalRacks + 1
' End If
' Next j
' Next i
'
MsgBox G
End Sub

Tommy
08-27-2009, 12:35 PM
First let me say that since you are counting columns along with rows what you are saying sounds reasonable.

Move the G counter outside of the column loop and see what happens.

mdmackillop
08-27-2009, 12:48 PM
Select case is better than complicated If statements. Also, build in checks for debugging.
It is not required to initialize Long variables to 0, however they should all be dimmed separately

Option Explicit
Private Sub CommandButton1_Click()
Dim i As Long, j As Long, nRow As Long, nCol As Long, G As Long
Dim Blank As Long
Dim xgg As Long, xgy As Long, xgr As Long, xyg As Long, xyy As Long, xyr As Long, xrg As Long, xry As Long, xrr As Long
nRow = 25
nCol = 32
' Check # times CFD predicts Green and ISX predicts Green, Yellow, or Red
With Worksheets("Best CI ISX")
For i = 1 To nRow
For j = 1 To nCol
If Worksheets("Best CI CFD").Cells(i, j) >= 0.9 Then
Select Case .Cells(i, j)
Case ""
Blank = Blank + 1 'for debug
'Do nothing
Case Is >= 0.9
xgg = xgg + 1
Case Is > 0.8
xgy = xgy + 1
Case Is <= 0.8
xgr = xgr + 1
End Select
G = G + 1
End If
Next j
Next i
End With
MsgBox G
MsgBox xgg & ", " & xgy & ", " & xgr & ", " & Blank & vbCr & xgg + xgy + xgr + Blank
End Sub

Tommy
08-27-2009, 12:50 PM
OK I was wrong try this to get the total number of cells that have data. This will get the total number of time it was red, green, or yellow, it does not count the number of times it wasn't a "color" or Worksheets("Best CI CFD").Cells(i, j) >= 0.9

Option Explicit
Private Sub CommandButton1_Click()

Dim i, j, nRow, nCol, G, Y, R, TotalRacks As Long
Dim xgg, xgy, xgr, xyg, xyy, xyr, xrg, xry, xrr As Long
Dim OverPredicted, MyError, Percent, WithInTenPercent As Double
Dim MyMax, ErrorSquare As Double


' *********************************************************************
' *********************************************************************
' Count # of times ISX is conservative or not


nRow = 25
nCol = 32

xgg = 0
xgy = 0
xgr = 0
xyg = 0
xyy = 0
xyr = 0
xrg = 0
xry = 0
xrr = 0
G = 0
Y = 0
R = 0

MyMax = 0
Percent = 0
OverPredicted = 0
WithInTenPercent = 0
ErrorSquare = 0
' Check # times CFD predicts Green and ISX predicts Green, Yellow, or Red
For i = 1 To nRow
For j = 1 To nCol


If Worksheets("Best CI CFD").Cells(i, j) >= 0.9 Then
If Worksheets("Best CI ISX").Cells(i, j) >= 0.9 Then
xgg = xgg + 1
G = G + 1
ElseIf Worksheets("Best CI ISX").Cells(i, j) < 0.9 And Worksheets("Best CI ISX").Cells(i, j) > 0.8 Then
xgy = xgy + 1
G = G + 1
ElseIf Worksheets("Best CI ISX").Cells(i, j) <= 0.8 And Worksheets("Best CI ISX").Cells(i, j) <> "" Then
xgr = xgr + 1
G = G + 1
End If

End If
Next j
Next i

MsgBox G
MsgBox xgr + xgy + xgg
End Sub

Saladsamurai
08-27-2009, 01:56 PM
OK I was wrong try this to get the total number of cells that have data. This will get the total number of time it was red, green, or yellow, it does not count the number of times it wasn't a "color" or Worksheets("Best CI CFD").Cells(i, j) >= 0.9

Option Explicit
Private Sub CommandButton1_Click()

Dim i, j, nRow, nCol, G, Y, R, TotalRacks As Long
Dim xgg, xgy, xgr, xyg, xyy, xyr, xrg, xry, xrr As Long
Dim OverPredicted, MyError, Percent, WithInTenPercent As Double
Dim MyMax, ErrorSquare As Double


' *********************************************************************
' *********************************************************************
' Count # of times ISX is conservative or not


nRow = 25
nCol = 32

xgg = 0
xgy = 0
xgr = 0
xyg = 0
xyy = 0
xyr = 0
xrg = 0
xry = 0
xrr = 0
G = 0
Y = 0
R = 0

MyMax = 0
Percent = 0
OverPredicted = 0
WithInTenPercent = 0
ErrorSquare = 0
' Check # times CFD predicts Green and ISX predicts Green, Yellow, or Red
For i = 1 To nRow
For j = 1 To nCol


If Worksheets("Best CI CFD").Cells(i, j) >= 0.9 Then
If Worksheets("Best CI ISX").Cells(i, j) >= 0.9 Then
xgg = xgg + 1
G = G + 1
ElseIf Worksheets("Best CI ISX").Cells(i, j) < 0.9 And Worksheets("Best CI ISX").Cells(i, j) > 0.8 Then
xgy = xgy + 1
G = G + 1
ElseIf Worksheets("Best CI ISX").Cells(i, j) <= 0.8 And Worksheets("Best CI ISX").Cells(i, j) <> "" Then
xgr = xgr + 1
G = G + 1
End If

End If
Next j
Next i

MsgBox G
MsgBox xgr + xgy + xgg
End Sub


Hello there :hi:

I see what you are doing here, but I do not see why all the G=G+1 's are necessary.

If you place only G = G+1 'in bewteen the 2 End If 's , why would it not count them? It is saying: ({} added for emphasis)

If Worksheets("Best CI CFD").Cells(i, j) >= 0.9 Then

{

If Worksheets("Best CI ISX").Cells(i, j) >= 0.9 Then
xgg = xgg + 1

ElseIf Worksheets("Best CI ISX").Cells(i, j) < 0.9 And Worksheets("Best CI ISX").Cells(i, j) > 0.8 Then
xgy = xgy + 1

ElseIf Worksheets("Best CI ISX").Cells(i, j) <= 0.8 And Worksheets("Best CI ISX").Cells(i, j) <> "" Then
xgr = xgr + 1

End If

}

G = G + 1
End If
Next j
Next i

MsgBox G
MsgBox xgr + xgy + xgg
End Sub

I am only trying to count the # of times that Worksheets("Best CI CFD").Cells(i, j) >= 0.9

So, if it meets the criteria of >=.9 it will step inside the nested if block and increment G. Right?

Saladsamurai
08-27-2009, 04:16 PM
Know what I'm saying? :*)

Bob Phillips
08-28-2009, 01:55 AM
Private Sub CommandButton1_Click()

Dim i, j, nRow, nCol, G, Y, R, TotalRacks As Long
Dim xgg, xgy, xgr, xyg, xyy, xyr, xrg, xry, xrr As Long
Dim OverPredicted, MyError, Percent, WithInTenPercent As Double
Dim MyMax, ErrorSquare As Double


' *********************************************************************
' *********************************************************************
' Count # of times ISX is conservative or not


nRow = 25
nCol = 32

xgg = 0
xgy = 0
xgr = 0
xyg = 0
xyy = 0
xyr = 0
xrg = 0
xry = 0
xrr = 0
G = 0
Y = 0
R = 0

MyMax = 0
Percent = 0
OverPredicted = 0
WithInTenPercent = 0
ErrorSquare = 0
' Check # times CFD predicts Green and ISX predicts Green, Yellow, or Red

For i = 1 To nRow

For j = 1 To nCol

If Worksheets("Best CI CFD").Cells(i, j) <> "" Then

If Worksheets("Best CI CFD").Cells(i, j) >= 0.9 Then

If Worksheets("Best CI ISX").Cells(i, j) >= 0.9 Then

xgg = xgg + 1
ElseIf Worksheets("Best CI ISX").Cells(i, j) > 0.8 Then

xgy = xgy + 1
Else

xgr = xgr + 1
End If

G = G + 1
End If
End If
Next j
Next i
MsgBox ">= .9 = " & xgg & vbNewLine & _
"> .8 = " & xgy & vbNewLine & _
"<= .8 = " & xgr & vbNewLine & _
"Total = " & G & vbNewLine
End Sub

Saladsamurai
08-28-2009, 05:09 AM
Private Sub CommandButton1_Click()

Dim i, j, nRow, nCol, G, Y, R, TotalRacks As Long
Dim xgg, xgy, xgr, xyg, xyy, xyr, xrg, xry, xrr As Long
Dim OverPredicted, MyError, Percent, WithInTenPercent As Double
Dim MyMax, ErrorSquare As Double


' *********************************************************************
' *********************************************************************
' Count # of times ISX is conservative or not


nRow = 25
nCol = 32

xgg = 0
xgy = 0
xgr = 0
xyg = 0
xyy = 0
xyr = 0
xrg = 0
xry = 0
xrr = 0
G = 0
Y = 0
R = 0

MyMax = 0
Percent = 0
OverPredicted = 0
WithInTenPercent = 0
ErrorSquare = 0
' Check # times CFD predicts Green and ISX predicts Green, Yellow, or Red

For i = 1 To nRow

For j = 1 To nCol

If Worksheets("Best CI CFD").Cells(i, j) <> "" Then

If Worksheets("Best CI CFD").Cells(i, j) >= 0.9 Then

If Worksheets("Best CI ISX").Cells(i, j) >= 0.9 Then

xgg = xgg + 1
ElseIf Worksheets("Best CI ISX").Cells(i, j) > 0.8 Then

xgy = xgy + 1
Else

xgr = xgr + 1
End If

G = G + 1
End If
End If
Next j
Next i
MsgBox ">= .9 = " & xgg & vbNewLine & _
"> .8 = " & xgy & vbNewLine & _
"<= .8 = " & xgr & vbNewLine & _
"Total = " & G & vbNewLine
End Sub


This actually a much cleaner way to do it. Me likes.

And though it pains me that no one can tell me what is wrong with the logic in the OP, I will succumb to the need for results.

Thank you mdmackillop, Tommy and xld for all of your help!

:beerchug:

Bob Phillips
08-28-2009, 05:20 AM
The error is shown in what I added. The cells contain formula, so the test for > 0.9 PASSES when a cell with that formula returns a blank, and gets counted in xgg.

Saladsamurai
08-28-2009, 05:24 AM
xld: Ok. So just to clarify, are you saying that Excel counts a FORMULA as being >= .9 ?

Bob Phillips
08-28-2009, 05:30 AM
no, I am saying that if the formula returns "" then VBA counts it as greater than any number



ActiveCell.Formula = "IF(" & ActiveCell.Offset(0, 1).Address & "<>""kll, kj2"","""",99)"
If ActiveCell > 10 ^ 10 Then MsgBox "yes"


You have to outsort them.

Saladsamurai
08-28-2009, 05:40 AM
Ok xld. So you are saying that

"" > 0.9

is TRUE in excel

Sorry if I am just asking the same question over again :o:

I am just kind of dumb lately...

Bob Phillips
08-28-2009, 05:47 AM
Yes, try it and see

Saladsamurai
08-28-2009, 05:58 AM
Yes, try it and see

Wow. I just did. I opened a new workbook and did the following:

Sub TestBlankCells()
Dim i As Integer, j As Integer, G As Integer
G = 0
For i = 1 To 10
For j = 1 To 10
If Cells(i, j) > 0.9 Then
G = G + 1
End If
Next j
Next i

MsgBox G
End Sub


Now, on a brand new workbook, this will Message Box " 0 "

BUT, if you go into the worksheet and take the same 100-cell range and make it so that all the of the cells are empty as a result of a logical test, then it will message box "100" !!!!!

This is the stupidest thing I have ever seen!

Thanks! I never would have figured that out (at least before I get fired fo effing around with the same code for like a week!)

Saladsamurai
08-28-2009, 06:04 AM
Select case is better than complicated If statements. Also, build in checks for debugging.
It is not required to initialize Long variables to 0, however they should all be dimmed separately


While this thread is active: Can you tell me what happens if when I declare my vars all at once like this:

Dim x,y,z As Integer

Is it true that this is the same as writing:

Dim x As Variant, y As Variant
Dim z As Integer

?

Bob Phillips
08-28-2009, 07:48 AM
Yes it is. If you don't explicitly declare the type, they assume variant.

Saladsamurai
08-28-2009, 08:15 AM
Yes it is. If you don't explicitly declare the type, they assume variant.

I see. Strange though. It seems that if I am using "Option Explicit" VBA should not even allow



Dim x,y,z As Integer

Bob Phillips
08-28-2009, 09:28 AM
No, Option Explicit means that you must declare the variable, it doesn't mean that you must declare its type. That is Option Strict, which VBA doesn't support. VBA supports weak typing, not strong typing.