johniewalker229 / aforge

Automatically exported from code.google.com/p/aforge
Other
0 stars 0 forks source link

Histogram functions for MAX and MIN are not working correct #350

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create a Histogram with values [1 2 3 4]
2. Get the Max Value

What is the expected output? What do you see instead?

Expectation: 4

What version of the product are you using?

don't know. Tested with other code. Problem found in a codereview.

Please provide any additional information below.

Sources/Math/Histogram.cs
The code to calc min and max is:
// calculate min and max
            for ( i = 0; i < n; i++ )
            {
                if ( values[i] != 0 )
                {
                    // max
                    if ( i > max )
                        max = i;
                    // min
                    if ( i < min )
                        min = i;

                    total += values[i];
                }
            }

but:
 if ( i > max )
 {max = i;}
does not make sence. I think
 if ( values[i] > max )
 {max = values[i];}
would be correct

Original issue reported on code.google.com by t.go...@gmail.com on 16 Jul 2013 at 4:13

GoogleCodeExporter commented 8 years ago
>> Expectation: 4
But what did you get? 3?

Original comment by andrew.k...@gmail.com on 16 Jul 2013 at 4:17

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
yes: 3

Other case:
Histogram a = new Histogram(new int[] {1, 10, 5});
Console.WriteLine(a.Max);

Expectation: 10
But was: 2

Original comment by t.go...@gmail.com on 16 Jul 2013 at 4:26

GoogleCodeExporter commented 8 years ago
RTFM then:
http://www.aforgenet.com/framework/docs/html/61212d57-52ee-9935-2364-b3a34a2213d
0.htm
and here
http://www.aforgenet.com/framework/docs/html/74b36509-b326-a6c1-61f9-dc906fb1b16
2.htm

In short, what you get is what is supposed/expected to be.

Original comment by andrew.k...@gmail.com on 16 Jul 2013 at 8:43