Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am working on a js code snippet to make a function which can be called with object literal.

Problem area : If i do not need to track the product value then a check has been placed for undefined. But do i really need to make check for each & every value if its not there in the object literal. Below is the code.

trackProduct = function (args) {

      if(args.label == undefined) {
        value.push([args.category, args.action, args.value]);
      } else if(args.value == undefined){
        value.push([args.category, args.action, args.label]);
      } else {
        value.push([args.category, args.action, args.label, args.value]);
      }
};

Calling this function via below object literal. I have not passed the value parameter.

trackProduct({
    category: elemcategory,
    action: elemaction,
    label: elemlabel
});

Do we need to check for Undefined for all three paramters? Any suggestions on how to improve this code.

share|improve this question
    
Are you using jQuery? –  Florian Margaine Nov 21 '12 at 10:43
    
You are using array to store different things, that doesn't make sense. You should just value.push( args ) –  Esailija Nov 21 '12 at 10:44
add comment

1 Answer

First, just a note: You check against undefined, but undefined is unfortunately not a keyword, meaning it's mutable. Better to use typeof something === 'undefined'

If I understand you right, you want to be able to "leave out" anyone of the object values. In that case, here's my take on the code (I'm returning an array, just for clarity)

function trackProduct(obj) {
  var keys = ['category', 'action', 'label', 'value'],  // properties to look for
      values = [],
      i, l, value;

  for(i = 0, l = keys.length ; i < l ; i++) {
    value = obj[keys[i]];
    if(typeof value !== 'undefined') {
      values.push(value); // if the value's there, put it in the array
    }
  }

  return values;
}

This would work like so:

trackProduct({
  category: "foo",
  label:    "bar",
  value:    "23",
  ignored:  "something"
}); // => ["foo", "bar", "23"]
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.