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

An option for .get to return undefined instead of throwing if pointer does not exist #19

Open
epoberezkin opened this issue Feb 17, 2016 · 6 comments

Comments

@epoberezkin
Copy link
Collaborator

@manuelstofer what do you think?
Or you prefer that users just try/catch?

@epoberezkin epoberezkin changed the title An option for .get to return undefined instead of throwing if path does not exist An option for .get to return undefined instead of throwing if pointer does not exist Feb 17, 2016
@manuelstofer
Copy link
Owner

I'm ok with returning undefined instead of throwing an exception. We should increase the version and add a note about backwards compatibility

@epoberezkin
Copy link
Collaborator Author

I think breaking backward compatibility is not a good idea, given how many packages use json-pointer. I already have tests that expect this exception. I think others will have a problem too, even if it's a major version change.

I would look at the option(s) in get, set, remove and api methods. I will think a bit and suggest something.

@futurist
Copy link

futurist commented Jun 3, 2016

Throwing error is really annoying. I think add a new option is just for backward compatibility, e.g.

api.get = function get (obj, pointer, silent) {
  .....
  if(!silent) throw new Error('Invalid reference token: ' + tok);
  ....
}

For old code, silent===undefined, and will throw error;
For new code, author can pass silent=true, and get undefined.

It's the same for get, remove, parse.

Hoping a version bump!

@evan-king
Copy link

Having to deal with thrown errors is not ideal for performance or brevity, nor necessarily is the vagueness of getting undefined and not knowing if the content did not exist or existed with value undefined.

However users have the necessary power as-is to efficiently provide the desired behavior for themselves. If you want silent failure, just write a wrapper function incorporating api.has():

function safeGet(obj, pointer) {
    return api.has(obj, pointer)
        ? api.get(obj, pointer)
        : undefined;
}

I'm not sure that adding bloat to the underlying library is warranted.

@epoberezkin
Copy link
Collaborator Author

epoberezkin commented Nov 21, 2016

@evan-king The reason to add something to the library is performance - the pointer in the wrapper above will be parsed and processed twice.

@danrot
Copy link

danrot commented May 8, 2019

I would prefer if get does not throw an error in case the property does not exist... As well as for performance and practical reasons. I also think that this is a case that happens quite a lot, and therefore is not an exceptional case (as throwing an exception should be).

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

5 participants