min/max, changes + tests#1
Conversation
Informaticore
left a comment
Informaticore
left a comment
There was a problem hiding this comment.
Hey I reviewed you PR - thank you very much for contributing!
| buffer[head] = value | ||
| head = (head + 1) % size_ | ||
| count = min (count + 1) size_ | ||
| count = (min (count + 1) size_) |
There was a problem hiding this comment.
why you put brackets here? it is a method call, it is not really needed I think.
| maximum -> float: | ||
| return reduce : | a b | max a b | ||
|
|
||
| //@davidg238: what is this method for? |
There was a problem hiding this comment.
I just added a minimum methode for some reason to check 2 values to which is the smaller one. Is there a different solution you would suggest? Couldn't find a system call (I also was not checking very hard)
There was a problem hiding this comment.
Yeah, they are tucked away in core/utils: https://libs.toit.io/core/utils/library-summary#max(2%2C0%2C0%2C)
| class TestRingbuffer implements TestCase: | ||
|
|
||
| run: | ||
| // test_ringbuffer_empty //@davidg238: not calling assertException correctly? |
There was a problem hiding this comment.
Yes - the assertException method is a bit confusing because Exception can be anything. You basically call the method first and catch the exception, then you check the exception like:
exception := catch:
ringbuffer.average
assertException exception
I just realized that I used it incorrectly myself in the test for the limit. I think the assertException can be done smarter where you give it a block which the method will run and check
There was a problem hiding this comment.
It looked like the test framework (love the name) was young, so I flagged it rather than attempt to fix it. A block would be preferable to simplify use.
| assertEquals 2.3 ringbuffer.average | ||
| assertEquals 2.3 ringbuffer.minimum | ||
| assertEquals 2.3 ringbuffer.maximum | ||
| // assertEquals 0.0 ringbuffer.std_deviation //@davidg238: should entries be required? |
There was a problem hiding this comment.
I think the deviation can only be calculated if there are at least two entries. so the deviation should be simply return 0.0 if it is called with less then 2 entries, right?
There was a problem hiding this comment.
Not do the calculation and return 0.0 ... yes.
2 participants
Added min/max, made suggestions, added tests but do not understand assertException ... see new tests