Calculate percentage of values in an array with a certain condition

Question

I had to write a function that calculates the percentage of values in table less or equal to 2. Is my code right or is there anything missing?

Public Function intPercentage As Integer
 Dim intAge( , ) As Integer = {{0, 2, 5, 10}
 {2, 0, 1, 3}
 {5, 1, 0, 6}
 {10, 3, 6, 0}}
Dim intRow, intCol As Integer
For intRow= 0 to 3
 For intCol = 0 to 3
 If intAge(intRow, intCol) >= 2 Then
 intTotal = intTotal + intAge(intRow, intCol)
 Next intCol
 Next intRow
 intPercentage = intTotal / 100
 End Function

• Jeroen Vannevel: Does it work for all test cases you can imagine? What test cases did you imagine?
• shaka: @JeroenVannevel the calculation? yes the calculation works fine afaik, i'm more interested in finding out of anyone could pick out anything wrong with my code, just a second opinion
• rolfl: This question confuses me. The description says "calculates the percentage of values in table less or equal to 2" but the code says: If intAge(intRow, intCol) >= 2 Then intTotal = intTotal + intAge(intRow, intCol). Two things, 1. are you looking for ages less than 2, or greater, and 2. are you averaging their ages, or getting the % of ages above/belop the 2 threshold?

Answer 1

Naming

Kill the Hungarian Notation. Kill it with fire. Don't bother taking it out back to bury it.

kill it with fire

Hungarian notation can be done right, but using it to tell yourself what datatype a variable is is not doing it right. Even if you must just insist on using hungarian notation this would not be right.

Dim intAge( , ) As Integer = {{0, 2, 5, 10}

intAge is not an integer. It is an array of arrays of integers.

Functions/Subs should have Verb-Noun style names. They should also be PascalCased.

Indentation

It's important. Like, really really important. Proper indentation makes it orders of magnitude easier to read code. Considering code will be read many more times than it will be written, it's important to make sure people can read it.

Everything inside of Function...End Function should be indented one level. Everything inside of For...Next should be indented one more level. Same goes for If...End If blocks.

Getting Down to Business

• A percentage is a double, not an integer.

  Public Function intPercentage As Integer
  

  If you want an integer representation of the percentage, you should probably leave a comment explaining why.

• A function isn't much good if it will return the same results every time. This one will because you've hardcoded the array of arrays into the function. It would be much more useful to accept a 2-dimensional array as an argument instead.

• I'm not personally a fan of declaring multiple variables on a single line, but there's technically nothing wrong with it.

  Dim intRow, intCol As Integer
  

• Since we're now going to accept an array as an argument, we'll no longer be able to hardcode For row = 0 to 3. You'll need to call GetLength on the internal array and Length on the outer one. (Example will be below.)

• You never declared intTotal. (Which you're going to call totalAge now, right?)

• Don't skimp on the End Ifs. Always be sure to close the If block unless you really write a one liner. (I don't recommend one liners by the way.)

• Prefer the Return keyword over assigning values to the function name.

• You can't just divide a number by 100 and call it a percentage.

  intPercentage = intTotal / 100
  

  In order to get the results you're really after, you need to divide the count of ages 2 or older by the count of all ages, then multiply by 100.

I took the liberty of re-writing the code.

Public Function GetPercentage(ages(,) As Integer) As Double
 Dim row As Integer
 Dim col As Integer
 Dim overTwo As Integer
 Dim total As Integer
 For row = 0 to ages.Length - 1
 total = total + ages.GetLength(row)
 For col = 0 to ages.GetLength(row) - 1
 If ages(row, col) >= 2 Then
 overTwo = overTwo + 1
 End If
 Next col
 Next row
 Return (overTwo / total) * 100
End Function

• RubberDuck: You're welcome. Out of curiousity, what exactly are you trying to return from this function?
• shaka: I had to get the percentage of values in table less or equal to 2
• RubberDuck: @shaka compared to the total number of entries in the table, correct?
• shaka: Yeah basically that
• RubberDuck: I updated my answer accordingly.