robtillaart:
I've been thinking about the issue with an even sized arrays. My understanding about medians from an even number of data points is it should be the mean of the two middle sorted data points. If you want to take this into consideration, what is the best way to determine an even number of data points from and odd number of data points?
[snip]fastest is the test for oddness
bool odd = _cnt & 0x01;
==>
bool even = !(_cnt & 0x01); // even = !odd;
Yeah, I'm feeling silly for not thinking of that.
[implementing even sized arrays correctly]
- if the elements are integers is the median also an integer? according to math it should be a float.
==> calculating the average is a float addition + division. + oddness test.
Well, a couple points here:
- The class defines all the elements, and the returns from the array, as float. Even if the user only puts ints (like returns from readAnalog()) into the array, they are stored as floats and the methods return floats. With nothing but integer-type values stored in the array, the only time a decimal value other than zero is returned would be a half. If the float return is silently cast into any of the integer-type datatypes, the half would be stripped, so the effect would be an automatic round-down. In the case of [2, 3], the rounded return would be the same as the current implementation. But in the case of [2, 5] a return of 2 isn't quite appropriate. The return should be 3.5 or rounded down to 3. How one is bothered by it depends on one's rounding practice. If one goes by the practice that exactly half (0.50...0) should be rounded down but anything over half (0.50...01) should be rounded up, then it isn't an issue.
- The oddness test only needs to be performed on _cnt which is uint8_t ("byte" in Adruino parlance). The addition and division may be simply be performed by returning the result of getAverage(2). Not sure how to have one method call another within a class though. My C++ isn't that strong.
- with odd array size the code works after the first element is added.
even arrays need at least 2 elements added.
True, but the getMedian method (as well as both the getAverage functions, but they are out of scope for this discussion) uses _cnt to determine the array size, not _size. Thus if only the first element has been added then the apparent array can't be anything but odd. (We already trap for zero, so that isn't an issue.) Unless things change in the class, this should be a non issue.
in short it adds code complexity to make it right.
I'll leave this point open for after the 0.1.06 version.
That's fair. No sense holding up something that (by the time I'm writing this) has already been released for something still under discussion. 