Skip to content

Added support for long in json#189

Merged
emweb merged 2 commits into
emweb:masterfrom
felix642:feature/jsonLong
Jun 28, 2022
Merged

Added support for long in json#189
emweb merged 2 commits into
emweb:masterfrom
felix642:feature/jsonLong

Conversation

@felix642

Copy link
Copy Markdown
Contributor

I don't see why JSON does not support long values.
I added a constructor that supports long values and the associated code.

@notorand-it

notorand-it commented May 13, 2022

Copy link
Copy Markdown

... and long long too?
Signed? Unsigned?

@felix642

Copy link
Copy Markdown
Contributor Author

Hi @notorand-it, long long was already supported. In fact the currently supported types are (long long, int and double).

I don't see why we are only restricting those types. When using the Wt::Json::Value class I expect to be able to give any primitive type and that the class should implicitly convert it to a number.
As of right now, I have to read to doc to see how to cast my value for it to be supported by the current implementation.

So, I do agree with you, I think that we should support unsigned numbers and signed numbers less than 4 bytes (short & int8_t).

Do you think there is a reason not to support them ?

@emweb emweb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution. I have some minor remarks before I can merge it.

As for why the JSON library may lack certain features: the Wt JSON library was created more out of necessity than as a feature of Wt. Wt::Auth's OAuth support needed to use JSON, so we added a JSON library back in 2011. We didn't have the wealth of JSON libraries that we have now, like JSON for Modern C++, Boost.JSON, or RapidJSON.

Regards,
Roel

Comment thread src/Wt/Json/Value.h Outdated
Comment thread src/Wt/Json/Value.h Outdated
@emweb emweb merged commit f4840ed into emweb:master Jun 28, 2022
@RockinRoel

Copy link
Copy Markdown
Contributor

Thanks. I like your profile picture 🙂

Regards,
Roel

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.

4 participants