-
Notifications
You must be signed in to change notification settings - Fork 8
Added Profile Load Estimator #134
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
Conversation
inc/Exceptions.h
Outdated
| /** @brief Defines a type of object to be thrown as exception. It reports | ||
| * errors that are due to out of range element access | ||
| */ | ||
| class OutOfRangeError : public std::out_of_range { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a benefit to this over throwing std::out_of_range directly?
src/Loading/ProfileLoadEstimator.cpp
Outdated
|
|
||
| uint32_t nElements = config.getUInt32(LOADING_LEN_KEY); | ||
|
|
||
| profile.resize(nElements); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To minimize the unnecessary construction of temporary elements, use profile.reserve(nElements) here and profile.emplace_back(duration_s, inputs) inside the loop.
src/Loading/ProfileLoadEstimator.cpp
Outdated
| } | ||
|
|
||
| LoadEstimator::LoadEstimate ProfileLoadEstimator::estimateLoad(const double t_s) { | ||
| const static double startT_s = t_s; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is doing what it's supposed to. This creates a single startT_s variable shared by all instances of the class. Should be replaced double and bool member variable.
| void registerConstLoadEstimatorTests(TestContext& context); | ||
| void registerGaussianLoadEstimatorTests(TestContext& context); | ||
| void registerMovingAverageLoadEstimatorTests(TestContext& context); | ||
| void profileTest() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the sake of consistency, shouldn't this go in its own file like all of the other individual tests?
No description provided.