2
\$\begingroup\$

I have a clear function that accepts an array, string, or no arguments. If no arguments, it should clear the entire data object. It works fine, but looking for a code review.

The possibilities should be:

clear('one');
clear(); //clear all
clear(['one', 'three']);

var data = {
  one: 'test1',
  two: 'test2',
  three: 'test3'
};

var clear = function(fields) {
  if (fields !== undefined) {
    if (typeof fields === 'string') {
      data[fields] = '';
    } else {
      _.each(fields, function(field) {
        if(data.hasOwnProperty(field)) {
          data[field] = '';
        }
      });
    }
  } else {
    // clear all
    _.each(data, function(value, key) {
      data[key] = '';
    });
  }
};

clear(['one', 'two']);
clear();
clear(['one']);
<script src="https://cdnjs.cloudflare.com/ajax/libs/underscore.js/1.8.3/underscore-min.js"></script>

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$
  1. The clear function relies on the global data variable. This could be more portable by passing the object in as the first argument, and then the fields as the second argument

    clear(data); // clear all
    clear(data, "one"); // clears the "one" field
    clear(data, ["one", "two"]); // clears multiple fields
    
  2. Checking if (fields !== undefined) will accept null values as a field name. A "truthy" check is probably what you need:

    if (fields) {
      // ...
    } else {
      // ...
    }
    
  3. What does "clear" mean? Your code assumes that all fields in an object should be cleared out to an empty string. What if the original value was a boolean, or number? Maybe you want to set that field to null instead. Better yet, maybe set it to null then delete the field:

    data[field] = null;
    delete data[field];
    

Other than that, it looks pretty good.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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