2

I am trying to test the truthiness of an object property. Whether it exists and has a value use that value and if it does not then add a default value to another object. 

const initNetwork = ( setupObj ) => {
    let obj = {};

    obj = Object.assign({}, setupObj);

    obj.eth0 = obj.eth0 ? obj.eth0 : {};
    obj.wlan0 = obj.wlan0 ? obj.wlan0 : {};

    obj.eth0.server = obj.eth0.server ? obj.eth0.server : {};
    obj.wlan0.client = obj.wlan0.client ? obj.wlan0.client : {};
    obj.wlan0.server = obj.wlan0.server? obj.wlan0.server : {};

    obj.eth0.mac = null;
    obj.wlan0.mac = null;

    obj.eth0.server.address = setupObj.eth0.server.address ? setupObj.eth0.server.address : "10.0.0.1";

}

initNetwork(); // intentionally leaving this empty to test setting default values.

I am getting an error here though. I thought it would return undefined and so would set obj.eth0.server.address to the false value of "10.0.0.1".

    obj.eth0.server.address = setupObj.eth0.server.address ? setupObj.eth0.server.address : "10.0.0.1";
                                       ^

TypeError: Cannot read property 'eth0' of undefined

What is the best way to see if this key/value pair exists all the way up the tree and if so use that value, otherwise set the false value?

shaun
  • 1,223
  • 1
  • 19
  • 44
  • It is telling you that `setupObj` doesn't exist, so you can't try to read the property `eth0` on it. – Scott Marcus Jan 31 '18 at 22:30
  • Why would you expect `setupObj` to be anything but `undefined` if you don't pass a value to `initNetwork()`? You're never reassigning `setupObj` – Patrick Roberts Jan 31 '18 at 22:30
  • I expect it to be the falsy `undefined` thus telling the ternary to use the false value. – shaun Jan 31 '18 at 22:31
  • 1
    @shaun You can't dereference a property of `undefined` – Alnitak Jan 31 '18 at 22:32
  • Not related, but note that `||` makes it shorter (once you get the undefined stuff resolved). `setupObj.eth0.server.address || "10.0.0.1";` –  Jan 31 '18 at 22:40
  • I think you should take a look at my answer below as it is better than the one you've accepted. – Scott Marcus Jan 31 '18 at 22:57

3 Answers3

3

You are assigning to a new object named obj so your check should be against obj.
In this situation setupObj will always be undefined.
You can add another condition for setupObj using the && operator: 

obj.eth0.server.address = (setupObj && setupObj.eth0.server.address) ? setupObj.eth0.server.address : "10.0.0.1";

Of course it is advised to check for each level of nested objects.

Running example: 

const initNetwork = ( setupObj ) => {
    let obj = {};

    obj = Object.assign({}, setupObj);

    obj.eth0 = obj.eth0 ? obj.eth0 : {};
    obj.wlan0 = obj.wlan0 ? obj.wlan0 : {};

    obj.eth0.server = obj.eth0.server ? obj.eth0.server : {};
    obj.wlan0.client = obj.wlan0.client ? obj.wlan0.client : {};
    obj.wlan0.server = obj.wlan0.server? obj.wlan0.server : {};

    obj.eth0.mac = null;
    obj.wlan0.mac = null;

    obj.eth0.server.address = (setupObj && setupObj.eth0.server.address) ? setupObj.eth0.server.address : "10.0.0.1";

}

initNetwork(); // intentionally leaving this empty to test setting default values.
Sagiv b.g
  • 30,379
  • 9
  • 68
  • 99
  • so this should work? `obj.eth0.server.address = setupObj && setupObj.eth0 && setupObj.eth0.server && setupObj.eth0.server.address ? setupObj.eth0.server.address : "10.0.0.1";` – shaun Jan 31 '18 at 22:36
  • @shaun if that situation can occur in your program, yes, you need to check your nested objects. – Ele Jan 31 '18 at 22:38
  • Given that `setupObj` needs to be tested twice and given all the other ternary operations, this really isn't a good solution. My answer below shows how it should be handled. – Scott Marcus Jan 31 '18 at 22:56
  • @ScottMarcus Given the complexity of the code you can take advantage of the es2015 `proxy` and create a function that safely extract a property. something like [this](https://medium.com/@chekofif/using-es6-s-proxy-for-safe-object-property-access-f42fa4380b2c) – Sagiv b.g Jan 31 '18 at 22:59
  • That's not really my point. Your solution still has 5 more ternary operators that aren't needed after it's determined that no argument was passed. It's not really practical to test all properties of all passed objects. In general, it's safe to assume that if an object was passed, then the object implements the correct interface for the receiving function. My point is that the rest of the function should be rewritten. – Scott Marcus Jan 31 '18 at 23:01
  • 2
    This is stack overflow, not [Code review](https://codereview.stackexchange.com/). the question is _"why is this ternary throwing an error instead of evaluating to false? "_. The OP got an answer + a simple example on how to prevent this kind of errors. If it was a code review situation, i couldn't say your code snippet is best practice. for example, your code will throw an error if we pass `1` -> `initNetwork(1)`. :) – Sagiv b.g Jan 31 '18 at 23:09
  • @Sagivb.g And yours will throw an error if we pass an object that has an `eth0` property, but not a `server` property of that of if there is no `address` property of `server`. My answer also identifies the issue and adds a more succinct explanation of how the rest of the code becomes redundant after addressing the main issue. – Scott Marcus Jan 31 '18 at 23:31
  • @ScottMarcus As i said, I'm not doing a code review here but trying to give a direct answer to the OP's question. I did mentioned in my answer that there should be a check for every nested object that is being accessed. – Sagiv b.g Jan 31 '18 at 23:39
0

You can test to see if an object exists, but you can't test to see if a property of a non-existent object exists and that's what your code attempts to do when setupObj is not passed in to the function.

You need your function to test to see if an argument was passed in before you attempt to use that argument. And, given that there are two spots in your code where you need to do this test, it makes more sense not to use the ternary and to instead use a traditional if.

The function should be as follows (you can actually run this code to see it in action):

const initNetwork = ( setupObj ) => {
    let obj = null;  // Don't set a value here because it's just going to be overridden

    // If setupObj exists....
    if(setupObj){
      // Use it:
      obj = Object.assign({}, setupObj);
      obj.eth0.server.address =  setupObj.eth0.server.address;
    } else {
      // But, if not, you need to set up your defaults:
      obj = {
         eth0: {
           server: {
             address: "10.0.0.1"
           },
           mac: null
         },
         wlan0 : {
            server: {},
            client: {},
            mac:null
         }
      };
    }
    
    // After that, you can proceed as you need to...
    
    // TEST:
    console.log(obj.eth0.server.address);
}

initNetwork(); // intentionally leaving this empty to test setting default values.

// Now a test for when a valid setupObj is passed:
var objTest = {
         eth0: {
           server: {
             address: "192.168.1.1"
           },
           mac: null
         },
         wlan0 : {
            server: {},
            client: {},
            mac:null
         }
      };

initNetwork(objTest);
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • 2
    Your code will throw an error if we pass `1` to the function `initNetwork(1);` – Sagiv b.g Jan 31 '18 at 23:11
  • @Sagivb.g Yes, of course it will. Just as most methods will throw errors when the wrong types are passed to them. So yes, a type check could be added. But, again, that's not the point that I'm making. I'm talking about the entire function needing a redesign. – Scott Marcus Jan 31 '18 at 23:27
0

Maybe a try / catch block can help you get rid of some extra code:

try {
    obj.eth0.server.address = setupObj.eth0.server.address;
} catch (e) {
    if (e instanceof TypeError) {
        obj.eth0.server.address = '10.0.0.1';
    } else {
        throw e;
    }
}
Miguel Calderón
  • 3,001
  • 1
  • 16
  • 18
  • `try/catch` should only be used in situations where the possibility of an error exists despite the developer doing everything possible to avoid it. It should not be used in lieu of more air-tight code. – Scott Marcus Jan 31 '18 at 23:29