0

I have some huge array structures and I have to traverse them and read some branches and sometimes have to modify some leaves.

The array keys I want to use are passed as an array themselves and generated somewhere else completely separated.

This is my current solution:

var arraytraverse = function(array, keys, value = undefined) {
  let pointer = array;
  for (let i = 0; i < keys.length; i++) {
    if (!keys.hasOwnProperty(i)) {
      continue;
    }
    if (i === keys.length - 1 && value !== undefined) {
      pointer[keys[i]] = value;
    }
    if (!pointer.hasOwnProperty(keys[i])) {
      return null;
    }
    pointer = pointer[keys[i]];
  }
  return pointer;
}

This is my jasmine test:

var data = {
  a: {
    b: {
      c: {
        foo: 'bar',
      },
    },
  },
};

describe('arraytraverse', () => {

  it('should be transparent', () => {
    let res = arraytraverse(data, []);
    expect(res).toEqual(data);
  });

  it('should be traversing', () => {
    let res = arraytraverse(data, ['a', 'b']);
    expect(res).toEqual(data.a.b);
  });

  it('should handle non existing', () => {
    let res = arraytraverse(data, ['a', 'miss']);
    expect(res).toBeNull();
  });

  it('should change by reference', () => {
    arraytraverse(data, ['a', 'b', 'c', 'foo'], 'baz');
    let res = arraytraverse(data, ['a', 'b', 'c', 'foo'], 'baz');
    expect(res).toBe('baz');
    expect(data.a.b.c.foo).toBe('baz');
  });

});

Everything is fine.

I just want to ask some more experienced of you if there is a way to improve my code.

Or are there better ways of doing it I am not aware of?

And no, I do not want to rely on any vendor. I want to learn. ;)

TIA!

edit: I added some changes to it: pastebin.com/UkUUSAZX

eurosat7
  • 103
  • 7
  • 3
    Try https://codereview.stackexchange.com/ this site deals with broken code – zer00ne Jan 22 '19 at 17:17
  • `!keys.hasOwnProperty(i)` seems like an odd condition. `i` is an integer, but in your example, the keys seem to be strings. – John Coleman Jan 22 '19 at 17:18
  • You are not working with array here, but with object. Your code actually reflects that as well as hasOwnProperty is only to be used with objects. – enf0rcer Jan 22 '19 at 17:19
  • @JohnColeman it checks if the array has an `i`-th element. That being said, it seems unnecessary. – Aurel Bílý Jan 22 '19 at 17:19
  • @zer00ne Thanks - wasn't aware of it! But my code ain't broken!!! ;-P – eurosat7 Jan 22 '19 at 17:26
  • @JohnColeman You are right. first check will be removed. – eurosat7 Jan 22 '19 at 17:26
  • @enf0rcer Sorry, you are right. I come from php and there it is just an associative array. I did not change my terminology. ;) – eurosat7 Jan 22 '19 at 17:27
  • @eurosat7 for your use case, I would advice you to use the Elvis operator(safe navigation pattern). More about it you can read here: https://www.beyondjava.net/elvis-operator-aka-safe-navigation-javascript-typescript – enf0rcer Jan 22 '19 at 17:28
  • The down side is browsers don't have it implemented yet. As the spec is still being worked on. But some clever workarounds to utilize the same way of thinking can be found in the article in the link – enf0rcer Jan 22 '19 at 17:29
  • more or less you have to traverse the structure in order to get to final key and get/set its value. So your code does just that, cannot be optimised more (ok maybe delete some check here and there), but unless js defines some new operation which can retrive deeply nested keys in an object this is the way to go – Nikos M. Jan 22 '19 at 17:39
  • I added some changes to it: https://pastebin.com/UkUUSAZX – eurosat7 Jan 22 '19 at 17:56

1 Answers1

1

Naming

  • arraytraverse should probably be objectTraverse.
  • array should be object.
  • pointer could perhaps be current? There are no pointers in JavaScript!

keys is an array

  • If you want to check if keys is an array, do it at the beginning of the function, and handle null or undefined; the keys.hasOwnProperty(i) check seems weird, since you already assume it is an array and take its length. You don't test for this in your test cases; and what would you do with keys like ["foo", null, "bar"]?
  • You could put keys.length into a const length = keys.length, then use that throughout the code.
  • You could also do ++i (may not be relevant anymore).

Last step

Every single iteration you check if you are at the end of the array yet and need to set a value (i === keys.length - 1 && value !== undefined). It might be better for your main loop to take one step less, then check if you need to insert the value after the loop, then finally do the last dereference current = current[keys[length - 1]], and finally check if it exists or return null.

Profile, profile, profile

JavaScript is optimised a lot in modern browsers. Some old tricks (like the ++i) may not work anymore, some "performance improvements" might actually decrease your performance nowadays. To test this, do some proper profiling with a large dataset to get some real metrics.

Aurel Bílý
  • 7,068
  • 1
  • 21
  • 34
  • Thanks a lot! I already integrated your first 6 tips. :) About the last step... I have to check how much performance loss that has - i do not want to double code. --- latest version here: https://pastebin.com/UkUUSAZX – eurosat7 Jan 22 '19 at 17:53
  • @eurosat7 Check out https://jsperf.com/, https://stackoverflow.com/questions/855126/what-is-the-best-way-to-profile-javascript-execution, and https://stackoverflow.com/questions/111368/how-do-you-performance-test-javascript-code – Aurel Bílý Jan 22 '19 at 17:57