Refactoring Javascript: Avoid Altering Arguments

Arguments passed in to a function should not be modified during the course of the function. If you need to modify the value of an argument, assign it to an appropriately named temporary variable first, and make your modifications there.

For example

Given an Employee constructor with start_data and end_date attributes:

function Person(last_name) {
	if (last_name.length > 4) {
		last_name = last_name.slice(0, 4);
	}
	this.last_name = last_name;
	return this.last_name;
}

Which might be used this way:

var person = new Person("Green");
person.last_name // ["Gree"]

It is confusing to see an argument change its value, especially in a language like Javascript where some arguments to a function are passed by value (eg: strings and numbers) and other arguments are passed by reference (eg: functions). Changing the value of a passed-in argument inside a function could change an external value in some cases but not in others.

A Jasmine test to make sure this is working might look like this:

describe("a person", function() {
	it("should have a last_name truncated to four letters", function() {
		var person = new Person("Green");
		expect(person.last_name).toEqual("Gree");
	});
});

Considerations:

If you are altering the value of the argument, you are creating something new, and that difference should be reflected in the name of the temporary variable.

If you are changing your approach so you no longer alter the value of a passed-in argument in code that is already working, you need to be careful that other references to the same argument will not break when you change the way you are handling it.

Because a function passed in as an argument may be used outside the context where it is being modified, you will need to do a much more complete analysis of where that passed-in function is being referenced outside the local context you are refactoring.

The highlighted code above could be refactored into:

function Person(last_name) {
	var truncated_last_name;
	if (last_name.length > 4) {
		truncated_last_name = last_name.slice(0, 4);
	}
	this.last_name = truncated_last_name;
};

By following these steps:

  1. Identify a passed-in argument that is altered over the course of a function.
  2. If you are using test-driven development, this is when you would define your first test parameters with an expected return value, and create your first test.
  3. Identify the state of the passed-in argument after it has been modified, and name a local temporary variable to reflect that state.
  4. Assign the value of the argument in the new state to the new temporary variable.
  5. Alter your code to reference the new temporary variable instead of the modified argument.
  6. Update any tests and test again to make sure things still work as expected.
  7. If you are changing the way you handle a passed-in function, be sure to check other uses of this same function outside the local scope.

This gets you a step closer to isolating your references, but I would also argue that it is better to avoid adding additional properties to your constructor that only serve to carry a temporary state.

Additional Options:

You can use getter and setter methods to handle references to a modified value that is based on an argument, and avoid even more confusion. If you make these methods public by attaching them to your constructor's prototype, you will be providing a new api for your constructor. That may involve changing how it is referenced externally and across your test suite. But it will improve the independence of your code, and make managing future changes easier.

function Person(last_name) {
	this.last_name = last_name;
};
Person.prototype.truncated_last_name = function(max) {
	var truncated_last_name;
	if (this.last_name.length > max) {
		truncated_last_name = this.last_name.slice(0, max);
	}
	return truncated_last_name;
};

Which might be used this way:

var person = new Person("Green");
person.truncated_last_name(4) // "Gree"

Keeping your arguments and the properties named after them from changing forces you to be explicit about how you use them.

An updated Jasmine test to make sure this is working might look like this:

describe("a person", function() {
	it("should have a truncated_last_name with four letters", function() {
		var person = new Person("Green");
		expect(person.truncated_last_name).toEqual("Gree");
	});
});

Arguments and Strict Mode

It's important to know that every Javascript function has an arguments object by default, which uses an array-like structure to carry the set of arguments that were passed in. Altering an argument alters the value maintained in that object. In strict mode, Ecmascript 5 will be much more particular about how you manipulate that object. If you're curious about why this issue is particularly relevant as Javascript evolves, you might want to read Angus Croll's 2011 post about the arguments object in Javascript functions and constructors.

It's not a bad idea to invoke strict mode as a general practice when refactoring a function, by adding the line "use strict"; at the beginning of the function. Strict mode will be invoked only for the local context of the function itself, and will enforce more careful coding practices. The invocation will be ignored in browsers not using Ecmascript 5 or better.

Post a comment

You may use the following HTML:
<a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>