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

as_float() doesn't take the default value #163

Closed
ilya1725 opened this issue Aug 31, 2017 · 7 comments
Closed

as_float() doesn't take the default value #163

ilya1725 opened this issue Aug 31, 2017 · 7 comments

Comments

@ilya1725
Copy link

I've tried passing the default values (not 0) to as_float() method described here. It doesn't seem to take it. The method still returns 0 when it can't convert the XML data.

@zeux
Copy link
Owner

zeux commented Sep 1, 2017

Just to make sure, are you referring to the case where the attribute is present, but contains values that do not represent a valid floating point number?

@ilya1725
Copy link
Author

ilya1725 commented Sep 1, 2017

Yes

@zeux
Copy link
Owner

zeux commented Sep 1, 2017

So in general this is currently working as documented (documentation says that conversion issues result in unspecified values). In terms of implementation, this is currently tracked as #15 (for integer parsing there's now custom validation code, but the behavior hasn't been changed to use the default value; for floating point parsing currently pugixml relies on strtod, that has some issues wrt error reporting).

Thinking about this, I realized that in one specific case the current behavior can be more surprising than usual, which is when the contents is empty. I could change the code to check that case specifically, although I'm not sure if in your case the value is empty as opposed to a non-empty string that just happens to not be a number.

@ilya1725
Copy link
Author

ilya1725 commented Sep 1, 2017

So I've tried two tests:
This is the data:
<index3 x='10' y='W178.314'/>
this is the code:
y.as_float(-12345.6) << ":'" << y.value()
the output:
0:'W178.314

With empty data:
<index3 x='10' y=''/>
output:
0:'

@zeux
Copy link
Owner

zeux commented Sep 1, 2017

Yeah, my question is whether fixing this for just empty data is interesting to you. The default values were originally intended for use when attributes just don't exist, i.e. <index3 x='10' /> in your example, which is why the current implementation only checks for that - but full validation is more complicated (#15 has some details)

@ilya1725
Copy link
Author

ilya1725 commented Sep 1, 2017

It didn't work for the invalid float either.
To be honest I don't really care, as long as the API does what the documentation says it should. It was my misunderstanding of the API. The documentation says "If attribute handle is null def argument is returned (which is 0 by default). " So the def technically only applicable to null handle and not the data itself.
It is up to you, this issue can be closed.

@zeux
Copy link
Owner

zeux commented Sep 6, 2017

Ok, for now I will just treat this as part of #15 then.

@zeux zeux closed this as completed Sep 6, 2017
@zeux zeux added the duplicate label Sep 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants