Behavior change (explicit throw)

[pocesar]

I'm inclined to throw an error (or an specialized error, like ObjectPathError) when the provided path is empty. It's usually a programming mistake, and it bit me a couple of times (usually when dealing with JSON coming from the server). Returning the original object shouldn't be the right thing to do, since it can be 'truthy'.

See #60, #59 and #54

Examples:

var ok;

Ok = 'some.path'; // mistyped the variable name
objectPath.set(obj, ok, 'some value'); 

/* ... */

$http.get('/endpoint').then(function(r) {
  if (objectPath.has(obj, r.path)) { // returns true even though r.path is undefined!
     return 'do something';
  }
  // it's actually in r.data.path, a return from angular $http, really easy to mess up
});

What is your opinion @mariocasciaro? I think it's the only place where the module should actually forcefully throw, to prevent hard-to-debug scenarios, since it will 'swallow' the empty path. And we shouldn't worry about it in the next version, since it's a major breaking change anyway

[mariocasciaro]

Should it throw for any empty path? [], "", null, undefined?

[pocesar]

yes, anything that isEmpty return as true

[mariocasciaro]

Ok, I think that might help with debugging, let's implement the new feature in the upcoming 1.0 and let's see how it goes.

[pocesar]

there's an edge case where there is no check, like objectPath.get(obj, [undefined]), the array isn't empty, but the path is invalid...

and going further, passing anything else isn't an object or an array in the first parameter should throw as well?

[xgbuils]

When path is empty array I like it as it is.

var obj = {
    foo: 'bar',
    fizz: {
        buzz: 42
    }
}
objectPath.get(obj, ['fizz', 'buzz']) // returns obj.fizz.buzz
objectPath.get(obj, ['foo']) // returns obj.foo
objectPath.get(obj, []) // returns obj

[pocesar]

the benefits of throwing is that you can mix, besides the obvious stack trace that helps a ton in debugging, you can catch them either in try / catch (including generators), promises catch calls

try {
   value = objectPath.get(obj, undefined);
} catch (e) {
   if (e instanceof ObjectPath.ObjectPathError) { // or e.name === 'ObjectPathError'
      value = objectPath.get(obj, nonundefined);
   }
}

// within promises
(new Promise(function(resolve){
   resolve(objectPath.get(obj, undefined)); // it can throw inside
}))
.then(function(value){
})
.catch('ObjectPathError', function(){
});

// within generators
function* oops(){ 
   objectPath.get(obj, undefined);
}
try {
  oops().next();
} catch (e) { 
  // [ObjectPathError: provided path is empty]
}

generators by itself are awful with stack traces, and proper exception throwing is a life saver

[xgbuils]

Ok, but my objection is with empty array. I think that is interesting to have a neutral element that it does nothing like empty array.

// now, if returns empty array
var path = createPathArrayBasedOnAnyTypeOfParameters(...)
objectPath.set(obj, path, 'foo') // do nothing

// Your proposal if returns empty array
var path = createPathArrayBasedOnAnyTypeOfParameters(...)
if (path.length > 0) {
    objectPath.set(obj, path, 'foo')
}

// or
try {
    objectPath.set(obj, path, 'foo')
} catch (e) {}

More examples that follow this pattern without throwing any error:

[].forEach(function () { ... })
[].indexOf(function () { ... })
[].every(function () { ... })

[pocesar]

but why would you be oblivious why your code is not executing? not everyone make testable code or rely solely on console.log for debugging. people tend to be pretty sloppy error handling, they slap their code around and expect it to work. object-path should make it easier to bypass a pain point of javascript, that is, to ACCESS and SETTING deep properties, if you are not using paths properly, you might just do if (!obj[someEmptyArray]){ } and it will pass the condition without trying an error

[xgbuils]

It is just my opinion, but access 0-deeply make sense for me and simplifies the code. Another example is with insert and push methods which access and modify element accessed:

Now:

var arr = []
var path = createPathArrayBasedOnAnyTypeOfParameters(...)
objectPath.push(arr, path, 5) // now: arr === [5]

Your proposal:

var arr = []
var path = createPathArrayBasedOnAnyTypeOfParameters(...)
if (path.length > 0) {
    objectPath.push(arr, path, 5)
} else {
    arr.push(5)
}

The people might make a mistake when don't assign value to a path variable. But if people pass variable with empty array value and don't know what that means, I think that it is their problem.

[pocesar]

ok that makes sense, for the sake of simplicity. maybe test if path is an array, and then don't test it if it's empty, shouldn't change anything in current tests and outcome.

[pocesar]

@mariocasciaro I noticed that if we don't pass a intermediate flag to the recursive calls, the path setting of value for non-existing paths 'throws'. Like, the intermediate non-existing path of course will be an empty {}, and it throws at the second recursive call. how to deal with this?