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

Use instance instead of JsonConvert class #14

Closed
ghost opened this issue Jun 14, 2017 · 5 comments
Closed

Use instance instead of JsonConvert class #14

ghost opened this issue Jun 14, 2017 · 5 comments
Assignees

Comments

@ghost
Copy link

ghost commented Jun 14, 2017

Hello,

I just found this package and I'd like to thank and congratulate you for your work.
However I've got a few concerns before it actually matches all my needs. Two of these concerns are already covered by issues #4 and #6 . My last (minor) concern is about converter configuration.

Here is what bothers me.
In my project there are two kinds of JSON that I will use. In one of them, the null values are perfectly fine. In the other one, null values are forbidden. And since the configuration is set at JsonConvert class level, I need to set the value checking mode at each use.

For instance:

/* kind1.service.ts */
// ...
// Each JSON conversion:
JsonConvert.valueCheckingMode = JsonConvert.ValueCheckingMode.ALLOW_NULL;
let obj: Kind1Object = JsonConvert.deserializeString(jsonString, Kind1Object );
// ...

/* kind2.service.ts */
// ...
// Each JSON conversion:
JsonConvert.valueCheckingMode = JsonConvert.ValueCheckingMode.DISALLOW_NULL;
let obj: Kind2Object = JsonConvert.deserializeString(jsonString, Kind2Object );
// ...

It means that my code spends time resetting the configuration every single time I use the converter.
I think it could be interesting to consider moving all the configurations and the methods to instances.
For example:

/* kind1.service.ts */
// ...
// Only once:
let kind1Converter: JsonConverter = JsonConvert.getConverter();
kind1Converter.valueCheckingMode = JsonConvert.ValueCheckingMode.ALLOW_NULL;
// ...
// Each JSON conversion:
let obj: Kind1Object = kind1Converter.deserializeString(jsonString, Kind1Object );
// ...

/* kind2.service.ts */
// ...
// Only once:
let kind2Converter: JsonConverter = JsonConvert.getConverter();
kind2Converter.valueCheckingMode = JsonConvert.ValueCheckingMode.DISALLOW_NULL;
// ...
// Each JSON conversion:
let obj: Kind2Object = kind2Converter.deserializeString(jsonString, Kind2Object );
// ...

This issue is probably not of the highest priority, and could be discarded completely, but I thought I should share with you my perspective on things.

@andreas-aeschlimann andreas-aeschlimann self-assigned this Jun 14, 2017
@andreas-aeschlimann
Copy link
Member

I'll close this as this is implemented now. Will publish the NPM package when the other features are included as well.

@ghost
Copy link
Author

ghost commented Jul 20, 2017

Thanks a lot. Congratulations for your 1.0.0 release !

@andreas-aeschlimann
Copy link
Member

Thanks, but no release done yet. We are still testing some things ;-)

Have a look at it, if you like to.

@ghost
Copy link
Author

ghost commented Jul 21, 2017

Haha, I hadn't noticed it wasn't really released yet. It's a major step in the right direction anyway. I just wanted to congratulate you :)
I probably will be using json2typescript at work in a very near future. I'll let you know if there are any issues, and I'll let my colleagues know I found a great JSON parsing library for Typescript ;)
I wish you the best for the future !

@andreas-aeschlimann
Copy link
Member

Yes we'll hopefully be able to release it soon.

Cheers

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

1 participant