VBA: Am i doing something wrong with While...wend?

[HeloNatebour]

Hello fellows, i need to do an average with an indeterminate list of numbers, then how many numbers are greater than the average and how many of them are less than the average.
I know this is a very newbie question but im still learning, so thanks in advance!
Here's my code:


Sub numb()
Dim DataRow,i As Integer
Dim avg, sum, count1, count2, count3 As Double
DataRow = 2
Do While Cells(DataRow, "A").Value <> ""
count1 = count1 + 1
Sum = sum + Cells(DataRow, "A").Value
DataRow = DataRow + 1
avg = sum / count1
Range("F3").Value = avg
Loop
'From here i dont know if i need to keep the If inside the While or use another While...
'I already have the avg at this point...
If Cells(DataRow, "A").Value >= avg Then
count2 = count2 + 1
ElseIf Cells(DataRow, "A").Value < avg Then
count3 = count3 + 1
End If
End Sub

[大灰狼1976]

Hi HeloNatebour!
Welcome to Vbax forum.
I like this way:

Sub numb_1()
Dim i&, arr, avg#, count1&, count2&
arr = Range("a2:a" & Cells(Rows.count, 1).End(3).Row)
avg = Application.Average(arr)
For i = 1 To UBound(arr)
If arr(i, 1) >= avg Then count1 = count1 + 1 Else count2 = count2 + 1
Next i
MsgBox "Greater then average: " & count1 & Chr(10) & "Less then average: " & count2
End Sub
or

Sub numb_2()
Dim count&, count1&, rng As Range
Set rng = Range("a2:a" & Cells(Rows.count, 1).End(3).Row)
count = Application.CountA(rng)
count1 = Application.CountIf(rng, ">=" & Application.Average(rng))
MsgBox "Greater then average: " & count1 & Chr(10) & "Less then average: " & count - count1
End Sub

[snb]

@1976

Please use indentation.


Sub M_snb()
Columns(1).SpecialCells(2, 1).Name = "snb"

MsgBox "average: " & [average(snb)] & vbLf & "smaller: " & [countif(snb,"<"&average(snb))] & vbLf & "larger: " & [countif(snb,"<"&average(snb))]
End Sub

[Paul_Hossler]

There are much better ways to do this, but this appears to be a learning exercise

I added some comments / suggestions /personal style to the macro for you to consider

I didn't change variable names, but I like to use self-documenting (aka 'descriptive') names, such as CountCells instead of count1, CountAboveAverage instead of count2,
CountBelowAverage instead of count3, etc.



Option Explicit

Sub numb()
'Dim DataRow, i As Integer -- Use Long
'Dim avg, sum, count1, count2, count3 As Double -- first 4 are Variant, since you did not explictly Dim them
Dim DataRow As Long, i As Long
Dim avg As Double, sum As Double, count1 As Double, count2 As Double, count3 As Double

DataRow = 2

' personally, I don't like to just let a macro assume ActiveSheet - better to be specific
With Worksheets("Sheet1")

'you can have a 0 length string, which is not the same as no data
'add a dot to objects to refer to the parent (Sheet1 in this case)
Do While Len(.Cells(DataRow, "A").Value) > 0
'there might be strings of strings that look like numbers, but aren't
If IsNumeric(.Cells(DataRow, "A").Value) Then
count1 = count1 + 1
sum = sum + .Cells(DataRow, "A").Value
DataRow = DataRow + 1
End If
'I think this should be the end of the first pass
Loop

avg = sum / count1
Range("F3").Value = avg

'repeat the basic loop to count above or below
Do While Len(.Cells(DataRow, "A").Value) > 0
'there might be strings of strings that look like numbers, but aren't
If IsNumeric(.Cells(DataRow, "A").Value) Then
If .Cells(DataRow, "A").Value >= avg Then
count2 = count2 + 1
ElseIf .Cells(DataRow, "A").Value < avg Then
count3 = count3 + 1
End If
End If
'I think this should be the end of the second pass
Loop
End With


End Sub

[snb]

@PH

VBA Lesson 2: reduce the amount of worksheet interaction to a minimum
VBA Lesson 3: always use Array variables the reduce this interaction

Another approach using Excel's builtin options:


Sub M_snb()
Cells(2, 3) = "=A2>average(A:A)"
Columns(1).AdvancedFilter 2, Cells(1, 3).Resize(2), Cells(1, 5)

Cells(2, 3) = "=A2<average(A:A)"
Columns(1).AdvancedFilter 2, Cells(1, 3).Resize(2), Cells(1, 7)
End Sub

[大灰狼1976]

Hi snb!
Thank you for your advice.:yes

[Paul_Hossler]

There are much better ways to do this, but this appears to be a learning exercise



@PH

VBA Lesson 2: reduce the amount of worksheet interaction to a minimum
VBA Lesson 3: always use Array variables the reduce this interaction




@snb

Yes, but

1. Those are not the 11th and 12th Commandments

2. The OP is learning, and I doubt that



Sub M_snb()
Columns(1).SpecialCells(2, 1).Name = "snb"

MsgBox "average: " & [average(snb)] & vbLf & "smaller: " & [countif(snb,"<"&average(snb))] & vbLf & "larger: " & [countif(snb,"<"&average(snb))]
End Sub


is easily understandable

3. The WS interaction delay is miniscule, and for things less than a gazillion cells, the improvement in wall clock time would most likely be imperceptible

4. Personally (bold, underlined) I think that the computer doesn't care if a variable is called 'snb' or 'CountOfCellsBelowAverage' but in 6 months, I'll know what it represents. I know you don't like to use "Option Explicit" and Dim everything as a Variant, but until the OP is as experienced as you are, every little bit helps

5. Possibly not important if the OP is VERY careful, Columns(1).SpecialCells(2,1) will fail if there are no constant numbers, or the wrong sheet is active


OP is of course able to choose whichever style he likes

[Aussiebear]

@ snb Not everybody likes to pretend that they are intergalatic interlecuals. You need to remember that there are many who operate at a lower level and are proud of it. Tone your attitude down please

[snb]

When is the beginning of your reading course ?