A shim to make make Node GRPC bindings more JavaScript-y

Posted on

Problem

This function allows me to write this code

const audioOutConfig = new AudioOutConfig();
audioOutConfig.setSampleRateHertz(16000);
audioOutConfig.setEncoding(AudioOutConfig.Encoding.LINEAR16);
audioOutConfig.setVolumePercentage(100);

as this code

const audioOutConfig = config(new AudioOutConfig(), {
  sampleRateHertz: 16000,
  encoding: AudioOutConfig.Encoding.LINEAR16,
  volumePercentage: 100
});

Is this a bad idea, and is the function I wrote to do it acceptable?

function config(obj, opts) {
  for (var opt in opts) {
    if (opts.hasOwnProperty(opt)) {
      obj[`set${opt.charAt(0).toUpperCase() + opt.slice(1)}`](opts[opt]);
    }
  }
  return obj;
}

Solution

const audioOutConfig = new AudioOutConfig();
audioOutConfig.setSampleRateHertz(16000);
audioOutConfig.setEncoding(AudioOutConfig.Encoding.LINEAR16);
audioOutConfig.setVolumePercentage(100);

There’s nothing wrong with the original implementation. That’s just how it’s done.

function config(obj, opts) {
  for (var opt in opts) {
    if (opts.hasOwnProperty(opt)) {
      obj[`set${opt.charAt(0).toUpperCase() + opt.slice(1)}`](opts[opt]);
    }
  }
  return obj;
}

const audioOutConfig = config(new AudioOutConfig(), {
  sampleRateHertz: 16000,
  encoding: AudioOutConfig.Encoding.LINEAR16,
  volumePercentage: 100
});

If you really want to configure it like the above, here’s a few things about your implementation. First, config is actually specific to AudioOutConfig. Not everything use set*-like methods. With that, passing in a new AudioOutConfig becomes redundant.

If config can be used on other objects, you risk executing methods that aren’t really there. If I put foo:'bar' on the config, it will try calling setFoo('bar') on your object which will fail. You’ll need to guard against that.

If you want to stick to the for-in loop, consider inverting the hasOwnProperty check and use continue. That way, there’s one less indent in the code.

In the end:

function config(obj, opts) {
  for (var opt in opts) {
    if (!opts.hasOwnProperty(opt)) continue;

    const methodName = `set${opt.charAt(0).toUpperCase() + opt.slice(1)}`
    if(!(methodName in obj)) continue;

    obj[methodName](opts[opt]);
  }
  return obj;
}

Both the idea and implementation look sound to me. As a nitpick, you might consider using a reduce on the opts, giving you a one liner. Also, I believe the hasOwnProperty test is superfluous here

Untested reduce version

Note that, per Joseph’s suggestion, which I agree with, this creates the AudioOutConfig object for you, rather than requiring it as a parameter:

function config(opts) {
  return Object.keys(opts).reduce((audio, opt) => {
    const methodName = `set${opt.charAt(0).toUpperCase() + opt.slice(1)}`
    return audio[methodName] ?  audio[methodName](opts[opt]) : audio
  }, new AudioOutConfig())
}

Leave a Reply

Your email address will not be published. Required fields are marked *