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

Should it throw an error if the property is not defined and defaultValue is missing? #43

Closed
stevemao opened this issue Apr 17, 2015 · 4 comments

Comments

@stevemao
Copy link
Contributor

expect(objectPath.get({}, 'deep.notDefined', undefined)).to.equal(undefined);

but

try {
  objectPath.get({}, 'deep.notDefined');
} catch (e) {
  assert(e.toString() === 'TypeError: Cannot read property \'notDefined\' of undefined');
}
@mariocasciaro
Copy link
Owner

IMO, one of the most useful aspects of object-path is the fact that it is "forgiving", it doesn't throw in case of missing properties. Personally, this is one of the main use cases of the library.

@stevemao
Copy link
Contributor Author

You could make it forgiving by providing a default value. if objectPath.get(obj, 'deep.notDefined') returns undefined user might assume that obj.deep exists hence there might be a bug. This would be an extra option for user

@pocesar
Copy link
Collaborator

pocesar commented Apr 17, 2015

well,

try {
   if (obj.deep.notDefined) { ... }
} catch (e) { }

does that just fine. #30 was looking for something like that, a "callbackish" solution for undefined paths, that you can definitely wrap in your own method, even using an unique identifier like NaN:

function getThrows(obj, path){
   var val = objectPath.get(obj, path, Number.NaN);
   // NaN is never equal to itself
   if (val !== val) {
     throw new Error(path.join('.') + ' is undefined');
   }
   return val;
}

and I agree with @mariocasciaro the highlight of this library is to safely access object properties without having to worry about exceptions being thrown and stopping the whole cycle (or shutting down a server, in case of node)

@pocesar
Copy link
Collaborator

pocesar commented Apr 19, 2015

if you expect something to throw synchronously, you just use try/catch and the code you expect to throw. with generators you can safely throw async, but that's not the case. nowadays everyone wants to cut error handling that is a basic and simple task.

var value;
if ((value = objectPath.get(obj, ['some.deep.path'])) {
   // deal with the value
} else {
  // move along
}

no pain and no effort in simple error handling. there's no need to throw there, there's no overhead since you are doing a simple get call and reusing the return if any, you are aware that your object might not be complete (the object might not have some.deep.path defined). and that's it. and that's the way to think ahead. if you can do proper error handling thinking ahead, you shouldn't be worrying about throwing from programming errors. the same goes with Promises, you know it might be rejected, and you know what to do with it, you just don't simply let it crash when it isn't supposed to be your fault (malformed objects, connection errors, global state inconsistency, etc).

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

3 participants