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

Suggestions #2

Open
miloyip opened this issue Sep 3, 2016 · 0 comments
Open

Suggestions #2

miloyip opened this issue Sep 3, 2016 · 0 comments

Comments

@miloyip
Copy link

miloyip commented Sep 3, 2016

  1. using namespace std in header file will pollute all implementation files that include the header. The practice is to use std::string, std::vector, etc. in header.
  2. Use size_t instead of int for size of array/object and indices. Otherwise there are a lot of related warnings.
  3. Currently, these APIs returns jValue by value.
jValue operator[](int i);
jValue operator[](string s);

This will make a deep copy of the whole subtree for every call.
If the library is designed for read-only access to the parse result (as I cannot find APIs for modifying existing values), I will suggest to change all APIs as const member functions:

string to_string() const;
jType get_type() const;
int as_int() const;
double as_double() const;
bool as_bool() const;
string as_string() const;
size_t size() const;
const jValue& operator[](size_t i) const ;
const jValue& operator[](const string& s) const;

While the other write-access API can be private and makes parser friend of jValue.

amir-s added a commit that referenced this issue Oct 17, 2019
#2 No 'using namespace' in header file. Fixed some size_t warnings.
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