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

apf::parameter_map is prone to bugs (with integer default arguments) #104

Open
mgeier opened this issue May 29, 2018 · 1 comment
Open

Comments

@mgeier
Copy link
Member

mgeier commented May 29, 2018

This was discovered there: #88 (comment)

Example:

apf::parameter_map params;
params.set("x", 3.14);
double good = params.get("x", 7.0);
double bad = params.get("x", 7);

The value of good will be 3.14.
The value of bad will be 7.0 because the get() function template will be instantiated for the int type deduced from the default argument 7, the contained value 3.14 will fail to be converted to int, therefore the default value 7 will be returned, which will get converted to 7.0 on assignment to the double variable.

I think it would be safer to disable the automatic type deduction, but this would mean that many uses of get() would have to be updated with an explicit template parameter like this:

auto no_problem = params.get<double>("x", 7);

The value of no_problem would then be 3.14, because the function template will be instantiated explicitly for the double type and the conversion of the stored value 3.14 would succeed. The default value 7 would be converted to double before calling the function, but it wouldn't be needed in this case.

I think this would be much safer than the status quo and it would avoid bugs like in the link given at the top, but I'm not sure if forcing an explicit template parameter is the best solution.

Any other suggestions?

@chris-hld
Copy link
Contributor

I agree, and I think it is also easier to grasp what actually happens by specifying the template parameter.

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

No branches or pull requests

2 participants