Support array notation in property path
leesei opened this issue · 15 comments
The doc said it is supported but actually not.
It is not covered in test.
const get = require('get-value');
const obj = { e: [ {f : 'g'} ] };
console.log(get(obj, 'e[0].f')); // undefined
console.log(get(obj, 'e.0.f')); // 'g'
I pushed up a branch with support for array-notation, but I haven't made up my mind on whether or not to support it, since it has a slight impact on performance.
how were you thinking of using this feature? what use case? It might help me decide. thanks!
I'm converting these to JSON in leesei/openslide-prop2json
.
openslide.level[0].downsample: '1'
openslide.level[0].height: '32893'
openslide.level[0].tile-height: '240'
openslide.level[0].tile-width: '240'
openslide.level[0].width: '46000'
openslide.level[1].downsample: '4.0000608050589808'
openslide.level[1].height: '8223'
openslide.level[1].tile-height: '240'
openslide.level[1].tile-width: '240'
openslide.level[1].width: '11500'
openslide.level[2].downsample: '16.003163017031632'
openslide.level[2].height: '2055'
openslide.level[2].tile-height: '240'
openslide.level[2].tile-width: '240'
openslide.level[2].width: '2875'
My current implementation also does not convert them to array.
Also, we must use array notation for hyphenated keys: object['foo-bar']: 3
.
Did you have the performance test set up?
I would like to try this:
var segs = prop.split(/[.\[\]]/).filter(function (s) { return s; });
Did you have the performance test set up?
yes, if you git clone then do npm install -D
, then run node benchmark
, you can see how changes effect the speed.
however... the regex you mentioned won't work since ].
is two characters. You'll need to add +
after the last bracket, like: /[.\[\]]+/
.
see https://github.com/jonschlinkert/get-value/blob/array-notation/index.js#L26
I'm guessing that the or(|
) in your regex causes backtracking.
I'm splitting ]
and .
(excessively) so I have to filter the empty segment.
I can try different regex and compare the performance. Thanks for the great work.
I'll get back to you.
no worries, thanks
http://jsperf.com/get-value-array-notation
Alas, I'm ashamed to put the "split and filter" code up, it only have 23000 ops.
Feel free to edit the test.
The performance was decreased with this added feature by more than 50%. I also hesitate to merge it to master.
I suggest adding an option for this use case.
var get = require('get-value')({arrayNotation: true});
index.js
would export closure that set a SPLIT_SEPERATOR
variable and return the current implementation with this modification:
var segs = prop.split(SPLIT_SEPERATOR);
Or we can allow user to pass the separator to support more use cases.
var GetValue = require('get-value');
var get1 = GetValue({separator: GetValue.DOT_ONLY}); // default
var get2 = GetValue({separator: GetValue.SUPPORT_ARRAY_NOTATION});
var get3 = GetValue({separator: '_'}); // custom separator
suggest adding an option for this use case.
Hmm, maybe we can take a function as the third argument, which would be expected to return an array of property paths.
thoughts?
That's great.
It provides the same feature without having to export a constructor function.
const get = require('get-value');
const obj = { e: [ {f : 'g'} ] };
// default tokenizer using '.'
// trying to access `obj['e[0]']`
console.log(get(obj, 'e[0].f')); // undefined
// custom tokenizer
// take cares of array notation
console.log(get(obj, 'e[0].f', (prop) => prop.split(/(?:\.|\[|\]\.)/)); // 'g'
Now that the a, b, c
params are kept, how should the function be added?
One way is to switch between the two features:
if (typeof a === 'function') {
tokenizer = a;
} else {
// allowing for multiple properties to be passed as
// a string or array, but much faster (3-4x) than doing
// `[].slice.call(arguments)`
if (a) prop += '.' + toString(a);
if (b) prop += '.' + toString(b);
if (c) prop += '.' + toString(c);
}
Closing since this was implemented a while ago.
@jonschlinkert Could you please give an example? I tried split and separator
options but cannot make it work. Thanks.
The README.md has an example for how to use the split
option. If that doesn't work, please open an issue with an example of the code your are trying.