Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added min/max functions #18

Merged
merged 2 commits into from
Apr 10, 2015
Merged

Added min/max functions #18

merged 2 commits into from
Apr 10, 2015

Conversation

muldereric
Copy link

Added getMin() and getMax() for a user of the library to easily keep
track of the lowest and highest value that were inserted in the
data-set.

Added getMin() and getMax() for a user of the library to easily keep
track of the lowest and highest value that were inserted in the
data-set.
@RobTillaart
Copy link
Owner

Thanks for this addition, I left those out of my original design to keep footprint minimal. Current compiler will optimize them out if not used, so I would like to add them.
However in case the _min and _max are both NAN the comparisons will produce false as wanted but comparing with NAN can be prevented which is imho more robust.

So I propose to change these lines

  • if (f < _min) _min = f;
  • if (f > _max) _max = f;
  • if (_cnt == 0) _min = _max = f;

to

  • if (_cnt == 0) _min = _max = f;
  • else if (f < _min) _min = f;
  • else if (f > _max) _max = f;

That will prevent comparing with NAN and reduce the average number of (float) compares.

Compare option could lead to comparing NAN with NAN. Though the outcome
is as expected, preventing the possibility is better.
@muldereric
Copy link
Author

As mentioned and long over due, I moved the code up 2 lines :)

RobTillaart added a commit that referenced this pull request Apr 10, 2015
@RobTillaart RobTillaart merged commit 33664e1 into RobTillaart:master Apr 10, 2015
@RobTillaart
Copy link
Owner

Note getMin() and getMax() gives the minimum/maximum since last call to clear(),
==> so it is not the min/max of the internal buffer.

  • refactored a bit to minimize footprint (0.2.08)
  • fixed some typos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants