Where, when, how, check for function arguments?. Best practices

help
1

Hello. I put a question on stackoverflow javascript best practices, http://stackoverflow.com/questions/33676880/on-javascript-code-where-when-how-check-function-arguments-best-practices . But I affraid that developers there don´t underestand me, my fall of course.

Well! I ask here cause I belong to here, Meteor, where I read many post of “on the trench” developers like @SkinnyGeek1010, @awatson1978, @arunoda, …

I had edited my code before put on stackoverflow cause is was a javascript questions but here I have the freedom to speak the Meteor way.

I have this functions:

// This is a private function, don´t check for undefined arguments.
function _arrayKeyDiff(a1, a2) {
  let r = {s:[], n: [], l: []};
  for(let k of a1) if (a2.indexOf(k) === -1) r.l.push(k); else r.s.push(k);
  for(let k of a2) if (a1.indexOf(k) === -1) r.n.push(k);
  return r;
}

// This a public fuction, check for isArray arguments.
arrayKeyDiff = function arrayKeyDiff(a1, a2) {
  return Array.isArray(a1) && Array.isArray(a2) && _arrayKeyDiff(a1, a2);
}

and this function that use public arrayKeyDiff

findDuplicateKey = function findDuplicateKey(...o) {
    // here 'o[0]' is checked cause Object.keys throw exception on undefined values
  if (!o[0]) return;
  let keys = [], acc = Object.keys(o[0]);

  for(let i = 1; i < o.length; i++) {
    let r;
    // 'o[i]' here again check before call Object.keys
    if (o[i]) {
      r = arrayKeyDiff(acc, Object.keys(o[i]));
      acc = acc.concat(r.n);
      r = r.s;
    }
    keys.push(r);
  }
  return keys;
}

and a ‘boolean’ return function that use ‘findDuplicateKey’ and console log mensage errors.

hasDuplicateKeyError = function hasDuplicateKeyError(...o) {
  let error = false;
  let r = findDuplicateKey(...o);
  r.forEach((v,i) => {
    if (v && v.length) {
     error = true;
     console.log('obj:', i, ' has duplicate key:', v);
    }
  });
  return error;
}

finally this function, ‘hasDuplicateKeyError’ is used on Match.Where check pattern like this:

nonHasDuplicateKeyObject = Match.Where(function(x) {
  check(x, [Match.Any]);
  return !hasDuplicateKeyError(...x);
});

// test code
const TestObj1 = { a: 1, b:1}, TestObj2 = { a:1, c:2 };
check([undefined, TestObj1, TestObj2], nonHasDuplicateKeyObject);

nonHasDuplicateKeyObject have a problem. If my first argument is undefined the call to ‘findDuplicateKey’ on function hasDuplicateKeyError, throw a exception. I refer to this line:

hasDuplicateKeyError = function hasDuplicateKeyError(...o) {
  let error = false;
  let r = findDuplicateKey(...o);
  r.forEach((v,i) => {

Here var ‘r’ is undefined cause this line on ‘findDuplicateKey’ function

findDuplicateKey = function findDuplicateKey(...o) {
    // here 'o[0]' is checked cause Object.keys throw exception on undefined values
  if (!o[0]) return;

Then I must be guard ‘r’ for ‘r.forEach’ cause it throw a exception is ‘r’ is undefined, then it need a ‘if’ like …

>     hasDuplicateKeyError = function hasDuplicateKeyError(...o) {
>       let error = false;
>       let r = findDuplicateKey(...o);
>       if (r)  r.forEach((v,i) => {

Well my problem is with ‘guards’. I start to put it all over the places, on any function before pass arguments to others functions. I lost to much time thinking on where put it and what check.

For example in function:

findDuplicateKey = function findDuplicateKey(...o) {
    // here 'o[0]' is checked cause Object.keys throw exception on undefined values
  if (!o[0]) return;
  let keys = [], acc = Object.keys(o[0]);

I have been thinking on replace

if (!o[0]) return;
let keys = [], acc = Object.keys(o[0]);

with

let o1 = o[0] || []; // default value []
let keys = [], acc = Object.keys(o1]);

And let the code continue on ‘undefined’ arguments passed but I don´t sure is this a good practice or bad or …

I think I miss something here, a good advice, a good pratice a, Lama mantra , …

Sorry for the huge wall text :frowning:

1 Like
2

I think it really depends on the context… If you’re app will break without the input you expect, you should throw an exception. check is an easy way to do this.

Also another approach to first filter or ‘clean’ the data before passing it into the other functions. This also gives you somewhat of a ‘schema’ of what the input data should ideally look like.
This is usually what I do if I have one bag of data that needs to go through a lot of functions. If you’re using all of these functions at once then I would use this path (personally).

// sample data
contacts = [{name: 'bob', number: null}, {number: '15550002222'}, {name: 'Jane', number: '15551112222'}]

// drop contacts without both name and number
_.chain(contacts)
.filter(c => !!c.name && !!c.number) // both exist returns true
.map(c => {number: '+' + c.number, name: c.name})
// other cleaning opts here
.value();

// or just fill with default values if you want to keep them
let cleanContacts = contacts.map(c => {
  return {
    name: c.name || '', 
    number: c.number || 'N/A'
  }
});

However, if you have lots of different data sources than perhaps ‘nerfing’ the functions is better. For example:

// using ES6 syntax


function arrayKeyDiff(a1 = [], a2 = []) {
  let r = {s:[], n: [], l: []};
  for(let k of a1) if (a2.indexOf(k) === -1) r.l.push(k); else r.s.push(k);
  for(let k of a2) if (a1.indexOf(k) === -1) r.n.push(k);
  return r;
}
3

Thanks @SkinnyGeek1010 for take time on this , all your advice are welcome.

A need a one more advice on this function:

findDuplicateKey = function findDuplicateKey(...o) {
    // here 'o[0]' is checked cause Object.keys throw exception on undefined values
  if (!o[0]) return;
  let keys = [], acc = Object.keys(o[0]);

  for(let i = 1; i < o.length; i++) {
    let r;
    // 'o[i]' here again check before call Object.keys
    if (o[i]) {
      r = arrayKeyDiff(acc, Object.keys(o[i]));
      acc = acc.concat(r.n);
      r = r.s;
    }
    keys.push(r);
  }
  return keys;
}

This function return an array or ‘undefined’ if first argument is undefined too. My recurrent problem is decide what return on this case ‘undefined’ or empty array []. Is a API design question? maybe I need a good lecture about.

In this case, ‘findDuplicateKey’ return three differents output an array, a empty array or undefined.
Some others functions return a object, a empty object or undefined.

On boolean return functions is easy return undefined like false. But return undefine like a empty array or empty object bring me to this point. is correct? is a good practice??

4

Yea I think so. In my opinion if you can return one type regardless of the input it’s generally a win. This was you don’t need to check before consuming it. If the function returns an array of duplicates, then an empty array makes sense if it doesn’t have duplicates or is called without arguments.

In this case, ‘findDuplicateKey’ return three differents output an array, a empty array or undefined.

Yea this is a code smell IMHO. a full array or empty array is ok but the undefined will cause possible explosions if this gets chained into a map or filter function for example.

5

is good to here that I can write a function that receive nothing(undefined) and return something([]) :slight_smile:

Get back to work.