My book on refactoring in 1999 began with a simple example of calculating and formatting a receipt for a video store. Modern JavaScript has several options for refactoring that code. Here I will outline four of them: refactoring of top-level functions; transition to a nested function with a dispatcher; using classes; transformation using an intermediate data structure.

Many years ago, when I was writing a book on refactoring , I started with a (very) simple example of refactoring code that calculated a bill for a client for renting video films (in those days we had to go to the salon for this). I recently thought about this example, in particular, how it would look like in modern JavaScript.

Any refactoring implies improving the code in a certain direction, in that which corresponds to the programming style of the development team. The example in the book was in Java, and Java (at that time) meant a certain programming style, an object-oriented style. However, with JavaScript, there are many more options to choose which style. Although you can adhere to a Java-like object-oriented style, especially with ES6 (Ecmascript 2015), not all JavaScript supporters approve of this style. Many people really think that using classes is Very Bad.

The initial code of the salon video rental

To continue the explanation, you need to show some code. In this case, the JavaScript version of the original example, which I wrote at the end of the last century.
')

function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { let movie = movies[r.movieID]; let thisAmount = 0; // determine amount for each movie switch (movie.code) { case "regular": thisAmount = 2; if (r.days > 2) { thisAmount += (r.days - 2) * 1.5; } break; case "new": thisAmount = r.days * 3; break; case "childrens": thisAmount = 1.5; if (r.days > 3) { thisAmount += (r.days - 3) * 1.5; } break; } //add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if(movie.code === "new" && r.days > 2) frequentRenterPoints++; //print figures for this rental result += `\t${movie.title}\t${thisAmount}\n` ; totalAmount += thisAmount; } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; } 

Here I am using ES6. The code works on two data structures, both of which are simply lists of json entries. The client record looks like this:

 { "name": "martin", "rentals": [ {"movieID": "F001", "days": 3}, {"movieID": "F002", "days": 1}, ] } 

The structure of the movie list is as follows:

 { "F001": {"title": "Ran", "code": "regular"}, "F002": {"title": "Trois Couleurs: Bleu", "code": "regular"}, // etc } 

In the original book, films were simply represented as objects in the structure of Java objects. For this article, I preferred to go to the json structure. It is assumed that some kind of global search such as Repository is not suitable for this application.

The method produces a simple text message about the video rental.

Rental Record for martin
Ran 3.5
Trois Couleurs: Bleu 2
Amount owed is 5.5
You earned 2 frequent renter points
This issue is pretty rough, even for an example. How could I not even bother to properly format the numbers? But remember that the book was written in the days of Java 1.1, before being added to the String format language. This partially justifies my laziness.

The statement function smells like Long Method . Only its size is already suspicious. But one bad smell is not a reason for refactoring. Badly factorized code is a problem because it is difficult to understand. If the code is difficult to understand, then it is difficult to change to add new features or correct errors. So if you don’t need to read or understand some code, then its bad structure doesn’t hurt you and you will be happy to leave it alone for a while. Therefore, to arouse our interest in this code, there must be some reason for changing it. Our reason, which I pointed out in the book, is to create an HTML version of the statement statement with something like this:

 <h1>Rental Record for <em>martin</em></h1> <table> <tr><td>Ran</td><td>3.5</td></tr> <tr><td>Trois Couleurs: Bleu</td><td>2</td></tr> </table> <p>Amount owed is <em>5.5</em></p> <p>You earned <em>2</em> frequent renter points</p> 

As I noted earlier, in this article I study some ways to refactor code in order to simplify the addition of additional rendering options for output. They all start in the same way: break one method into a set of functions, covering different parts of logic. When I finish this partition, I’ll explore four ways how these functions can be organized to support alternative rendering.

Splitting into several functions

Every time I work with too long a function like this, my first thought is to try to break it up into logical pieces of code and make separate functions of them using the Extract Method . [1] . The first piece that caught my attention was the switch .

  function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { let movie = movies[r.movieID]; let thisAmount = 0; // determine amount for each movie switch (movie.code) { case "regular": thisAmount = 2; if (r.days > 2) { thisAmount += (r.days - 2) * 1.5; } break; case "new": thisAmount = r.days * 3; break; case "childrens": thisAmount = 1.5; if (r.days > 3) { thisAmount += (r.days - 3) * 1.5; } break; } //add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if(movie.code === "new" && r.days > 2) frequentRenterPoints++; //print figures for this rental result += `\t${movie.title}\t${thisAmount}\n` ; totalAmount += thisAmount; } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; } 

My development environment (IntelliJ) offers itself to do refactoring automatically, but incorrectly conducts it - its abilities in JavaScript are not as advanced as in Java refactoring. So I will do it manually. You need to see what data this extract candidate uses. There are three pieces of data:

• The value of thisAmount calculated by the extracted code. I can initiate it inside a function and return it at the end.
• The value of r for the number of days of rental is checked in a cycle, I can pass it as a parameter.
• The variable movie is a rented movie . Temporary variables like this usually get in the way of refactoring procedural code, so I would prefer to run Replace Temp with Query first to convert it into a function that I can call from any extracted code.

When I finish with Replace Temp with Query , the code looks like this:

  function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { let thisAmount = 0; // determine amount for each movie switch (movieFor(r).code) { case "regular": thisAmount = 2; if (r.days > 2) { thisAmount += (r.days - 2) * 1.5; } break; case "new": thisAmount = r.days * 3; break; case "childrens": thisAmount = 1.5; if (r.days > 3) { thisAmount += (r.days - 3) * 1.5; } break; } //add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if(movieFor(r).code === "new" && r.days > 2) frequentRenterPoints++; //print figures for this rental result += `\t${movieFor(r).title}\t${thisAmount}\n` ; totalAmount += thisAmount; } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; function movieFor(rental) {return movies[rental.movieID];} } 

Now extract the switch .

  function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { const thisAmount = amountFor(r); //add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if(movieFor(r).code === "new" && r.days > 2) frequentRenterPoints++; //print figures for this rental result += `\t${movieFor(r).title}\t${thisAmount}\n` ; totalAmount += thisAmount; } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; function movieFor(rental) {return movies[rental.movieID];} function amountFor(r) { let thisAmount = 0; // determine amount for each movie switch (movieFor(r).code) { case "regular": thisAmount = 2; if (r.days > 2) { thisAmount += (r.days - 2) * 1.5; } break; case "new": thisAmount = r.days * 3; break; case "childrens": thisAmount = 1.5; if (r.days > 3) { thisAmount += (r.days - 3) * 1.5; } break; } return thisAmount; } } 

Now look at the calculation of regular customer points. Here you can perform the same extraction procedure.

 function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { const thisAmount = amountFor(r); frequentRenterPointsFor(r); //print figures for this rental result += `\t${movieFor(r).title}\t${thisAmount}\n` ; totalAmount += thisAmount; } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; … function frequentRenterPointsFor(r) { //add frequent renter points frequentRenterPoints++; // add bonus for a two day new release rental if (movieFor(r).code === "new" && r.days > 2) frequentRenterPoints++; } 

Although I learned the function, I don’t like how it works by updating the variable of the parent area. Such side effects make the code difficult, so I will change it to take away these side effects.

 function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { const thisAmount = amountFor(r); frequentRenterPoints += frequentRenterPointsFor(r); //print figures for this rental result += `\t${movieFor(r).title}\t${thisAmount}\n` ; totalAmount += thisAmount; } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; … function frequentRenterPointsFor(r) { let result = 1; if (movieFor(r).code === "new" && r.days > 2) result++; return result; } 

I will use the chance to slightly clean the two extracted functions while I understand them.

  function amountFor(rental) { let result = 0; switch (movieFor(rental).code) { case "regular": result = 2; if (rental.days > 2) { result += (rental.days - 2) * 1.5; } return result; case "new": result = rental.days * 3; return result; case "childrens": result = 1.5; if (rental.days > 3) { result += (rental.days - 3) * 1.5; } return result; } return result; } function frequentRenterPointsFor(rental) { return (movieFor(rental).code === "new" && rental.days > 2) ? 2 : 1; } 

With these functions, I could do something else, especially with the amountFor , and I really did something in the book. But for this article, I will no longer delve into the study of the body of these functions

Done, now back to the body of the function.

  function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { const thisAmount = amountFor(r); frequentRenterPoints += frequentRenterPointsFor(r); //print figures for this rental result += `\t${movieFor(r).title}\t${thisAmount}\n` ; totalAmount += thisAmount; } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; 

The general approach I like to use is to eliminate mutable variables. Here there are three, one collects the final line, two more calculate the values ​​that are used in this line. I have nothing against the first, but I would like to destroy the other two. First you need to break the cycle. First, simplify the cycle and embed a constant value.

  function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { frequentRenterPoints += frequentRenterPointsFor(r); result += `\t${movieFor(r).title}\t${amountFor(r)}\n` ; totalAmount += amountFor(r); } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; 

Then we divide the cycle into three parts.

  function statement(customer, movies) { let totalAmount = 0; let frequentRenterPoints = 0; let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { frequentRenterPoints += frequentRenterPointsFor(r); } for (let r of customer.rentals) { result += `\t${movieFor(r).title}\t${amountFor(r)}\n`; } for (let r of customer.rentals) { totalAmount += amountFor(r); } // add footer lines result += `Amount owed is ${totalAmount}\n`; result += `You earned ${frequentRenterPoints} frequent renter points\n`; return result; 

Some programmers are worried about performance issues after such refactoring, in which case look at the old, but relevant, article about software performance.

Such a partition allows you to later extract the functions for these calculations.

  function statement(customer, movies) { let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${movieFor(r).title}\t${amountFor(r)}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; function totalAmount() { let result = 0; for (let r of customer.rentals) { result += amountFor(r); } return result; } function totalFrequentRenterPoints() { let result = 0; for (let r of customer.rentals) { result += frequentRenterPointsFor(r); } return result; } 

As a fan of the collection pipeline, I also adjust cycles to such a chain.

  function totalFrequentRenterPoints() { return customer.rentals .map((r) => frequentRenterPointsFor(r)) .reduce((a, b) => a + b) ; } function totalAmount() { return customer.rentals .reduce((total, r) => total + amountFor(r), 0); } 

Not sure which of these two types of chains I like best

Investigation of the assembled function

Now let's see what we did. Here is the whole code.

 function statement(customer, movies) { let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${movieFor(r).title}\t${amountFor(r)}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; function totalFrequentRenterPoints() { return customer.rentals .map((r) => frequentRenterPointsFor(r)) .reduce((a, b) => a + b) ; } function totalAmount() { return customer.rentals .reduce((total, r) => total + amountFor(r), 0); } function movieFor(rental) { return movies[rental.movieID]; } function amountFor(rental) { let result = 0; switch (movieFor(rental).code) { case "regular": result = 2; if (rental.days > 2) { result += (rental.days - 2) * 1.5; } return result; case "new": result = rental.days * 3; return result; case "childrens": result = 1.5; if (rental.days > 3) { result += (rental.days - 3) * 1.5; } return result; } return result; } function frequentRenterPointsFor(rental) { return (movieFor(rental).code === "new" && rental.days > 2) ? 2 : 1; } } 

Now I have a well-composed function. Its main code takes seven lines, and they all relate to formatting the final line. The code for all calculations is transferred to its own set of nested functions, each of which is small and with a distinct name that indicates its purpose.

But I'm still not ready to write a function for issuing html. All functions after splitting are nested inside the general function statement . So it is easier to extract functions, since they can refer to the names inside the scope of the function, including each other (as amountFor call movieFor ) and the corresponding customer and movie parameters. But I can't write a simple htmlStatement function that references these functions. In order to support some other output formats using the same calculations, refactoring should be continued. Now I’ll open the dots when different refactoring options appear depending on how I want to convert the code. Then I will test each of these options, explain how each of them works, and when all four are ready, we will compare them.

Using the parameter to determine the issue

One option is to define the output format as an argument to the statement function. I would like to start this refactoring by using Add Parameter , extract the existing code to format the text, and append the code at the beginning to refer to the extracted function when the parameter indicates this.

 function statement(customer, movies, format = 'text') { switch (format) { case "text": return textStatement(); } throw new Error(`unknown statement format ${format}`); function textStatement() { let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${movieFor(r).title}\t${amountFor(r)}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; } 

Then I can write the html generation function and add a condition to the dispatcher.

  function statement(customer, movies, format = 'text') { switch (format) { case "text": return textStatement(); case "html": return htmlStatement(); } throw new Error(`unknown statement format ${format}`); function htmlStatement() { let result = `<h1>Rental Record for <em>${customer.name}</em></h1>\n`; result += "<table>\n"; for (let r of customer.rentals) { result += ` <tr><td>${movieFor(r).title}</td><td>${amountFor(r)}</td></tr>\n`; } result += "</table>\n"; result += `<p>Amount owed is <em>${totalAmount()}</em></p>\n`; result += `<p>You earned <em>${totalFrequentRenterPoints()}</em> frequent renter points</p>\n`; return result; } 

I can use the data structure for the controller logic.

 function statement(customer, movies, format = 'text') { const dispatchTable = { "text": textStatement, "html": htmlStatement }; if (undefined === dispatchTable[format]) throw new Error(`unknown statement format ${format}`); return dispatchTable[format].call(); 

Using top level features

The problem with writing the top-level statement function is that the evaluation functions are nested inside the statement function. The obvious way out is to move them to the upper context.

To do this, I began by searching for a function that does not refer to any others, in our case, movieFor .

Whenever I move functions, I like to first copy a function into a new context, embed it in that context, and then replace the body of the original function with a call to the moved function.

 function topMovieFor(rental, movies) { return movies[rental.movieID]; } function statement(customer, movies) { // [snip] function movieFor(rental) { return topMovieFor(rental, movies); } function frequentRenterPointsFor(rental) { return (movieFor(rental).code === "new" && rental.days > 2) ? 2 : 1; } 

At this stage, you can compile and test the code to check if there are any problems due to the change of context. When this is done, you can embed the forwarding feature.

 function movieFor(rental, movies) { return movies[rental.movieID]; } function statement(customer, movies) { // [snip] function frequentRenterPointsFor(rental) { return (movieFor(rental, movies).code === "new" && rental.days > 2) ? 2 : 1; } 

Similar changes are made inside amountFor

Simultaneously with embedding, I also renamed the top-level function to match the old name, so the only difference now is movies .

Then do this with all the nested functions.

 function statement(customer, movies) { let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${movieFor(r, movies).title}\t${amountFor(r, movies)}\n`; } result += `Amount owed is ${totalAmount(customer, movies)}\n`; result += `You earned ${totalFrequentRenterPoints(customer, movies)} frequent renter points\n`; return result; } function totalFrequentRenterPoints(customer, movies) { return customer.rentals .map((r) => frequentRenterPointsFor(r, movies)) .reduce((a, b) => a + b) ; } function totalAmount(customer, movies) { return customer.rentals .reduce((total, r) => total + amountFor(r, movies), 0); } function movieFor(rental, movies) { return movies[rental.movieID]; } function amountFor(rental, movies) { let result = 0; switch (movieFor(rental, movies).code) { case "regular": result = 2; if (rental.days > 2) { result += (rental.days - 2) * 1.5; } return result; case "new": result = rental.days * 3; return result; case "childrens": result = 1.5; if (rental.days > 3) { result += (rental.days - 3) * 1.5; } return result; } return result; } function frequentRenterPointsFor(rental, movies) { return (movieFor(rental, movies).code === "new" && rental.days > 2) ? 2 : 1; } 

Now I can easily write the htmlStatement function.

 function htmlStatement(customer, movies) { let result = `<h1>Rental Record for <em>${customer.name}</em></h1>\n`; result += "<table>\n"; for (let r of customer.rentals) { result += ` <tr><td>${movieFor(r, movies).title}</td><td>${amountFor(r, movies)}</td></tr>\n`; } result += "</table>\n"; result += `<p>Amount owed is <em>${totalAmount(customer, movies)}</em></p>\n`; result += `<p>You earned <em>${totalFrequentRenterPoints(customer, movies)}</em> frequent renter points</p>\n`; return result; } 

Declaring some local functions with partial use

When the global function is used in this way, the parameter lists can stretch out pretty heavily. So it can sometimes be useful to declare a local function that calls a global function with some or all of the parameters inside. This local function, which is a partial application of a global function, can be useful for later use. There are various ways to do this in JavaScript. One of them is to assign local functions to variables.

  function htmlStatement(customer, movies) { const amount = () => totalAmount(customer, movies); const frequentRenterPoints = () => totalFrequentRenterPoints(customer, movies); const movie = (aRental) => movieFor(aRental, movies); const rentalAmount = (aRental) => amountFor(aRental, movies); let result = `<h1>Rental Record for <em>${customer.name}</em></h1>\n`; result += "<table>\n"; for (let r of customer.rentals) { result += ` <tr><td>${movie(r).title}</td><td>${rentalAmount(r)}</td></tr>\n`; } result += "</table>\n"; result += `<p>Amount owed is <em>${amount()}</em></p>\n`; result += `<p>You earned <em>${frequentRenterPoints()}</em> frequent renter points</p>\n`; return result; } 

Another way is to declare them as nested functions.

  function htmlStatement(customer, movies) { let result = `<h1>Rental Record for <em>${customer.name}</em></h1>\n`; result += "<table>\n"; for (let r of customer.rentals) { result += ` <tr><td>${movie(r).title}</td><td>${rentalAmount(r)}</td></tr>\n`; } result += "</table>\n"; result += `<p>Amount owed is <em>${amount()}</em></p>\n`; result += `<p>You earned <em>${frequentRenterPoints()}</em> frequent renter points</p>\n`; return result; function amount() {return totalAmount(customer, movies);} function frequentRenterPoints() {return totalFrequentRenterPoints(customer, movies);} function rentalAmount(aRental) {return amountFor(aRental, movies);} function movie(aRental) {return movieFor(aRental, movies);} } 

Another option is to use bind . I leave it to you for your own research - this is not what I would use here, since the previous options seem to me more suitable.

Using classes

I am familiar with the object approach, so it is not surprising that I am going to consider classes and objects. ES6 has a nice syntax for the classic object approach. Let's see how to apply it in this example.

First of all we wrap the data in the objects, starting with the customer .

customer.es6 ...

  export default class Customer { constructor(data) { this._data = data; } get name() {return this._data.name;} get rentals() { return this._data.rentals;} } 

statement.es6 ...

  import Customer from './customer.es6'; function statement(customerArg, movies) { const customer = new Customer(customerArg); let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${movieFor(r).title}\t${amountFor(r)}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; 

Until now, the class is a simple wrapper around the original JavaScript object. Next we will do the same for rental .

rental.es6 ...

  export default class Rental { constructor(data) { this._data = data; } get days() {return this._data.days} get movieID() {return this._data.movieID} } 

customer.es6 ...

  import Rental from './rental.es6' export default class Customer { constructor(data) { this._data = data; } get name() {return this._data.name;} get rentals() { return this._data.rentals.map(r => new Rental(r));} } 

Now that classes are created around my simple json objects, a job for the Move Method has appeared . As during the transfer of functions to the upper level, first of all we take the function that does not apply to any other - movieFor . But this function needs a list of movies as a context that needs to be made available for the rental objects being created.

statement.es6 ...

  function statement(customerArg, movies) { const customer = new Customer(customerArg, movies); let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${movieFor(r).title}\t${amountFor(r)}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; 

class Customer ...

  constructor(data, movies) { this._data = data; this._movies = movies } get rentals() { return this._data.rentals.map(r => new Rental(r, this._movies));} 

class Rental ...

  constructor(data, movies) { this._data = data; this._movies = movies; } 

When I have all the supporting data in place, I can transfer the function.

statement.es6 ...

  function movieFor(rental) { return rental.movie; } 

class Rental ...

 class Rental... get movie() { return this._movies[this.movieID]; } 

As with the previous move, first of all, we move the key behavior of the function into a new context, embed it in the context, and configure the original function to call a new function. When this works, it is relatively easy to embed calls to the original function.

statement.es6 ...

 function statement(customerArg, movies) { const customer = new Customer(customerArg, movies); let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${r.movie.title}\t${amountFor(r)}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; function amountFor(rental) { let result = 0; switch (rental.movie.code) { case "regular": result = 2; if (rental.days > 2) { result += (rental.days - 2) * 1.5; } return result; case "new": result = rental.days * 3; return result; case "childrens": result = 1.5; if (rental.days > 3) { result += (rental.days - 3) * 1.5; } return result; } return result; } function frequentRenterPointsFor(rental) { return (rental.movie.code === "new" && rental.days > 2) ? 2 : 1; } 

rental .

statement.es6…

 function statement(customerArg, movies) { const customer = new Customer(customerArg, movies); let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${r.movie.title}\t${r.amount}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; function totalFrequentRenterPoints() { return customer.rentals .map((r) => r.frequentRenterPoints) .reduce((a, b) => a + b) ; } function totalAmount() { return customer.rentals .reduce((total, r) => total + r.amount, 0); } 

class Rental...

  get frequentRenterPoints() { return (this.movie.code === "new" && this.days > 2) ? 2 : 1; } get amount() { let result = 0; switch (this.movie.code) { case "regular": result = 2; if (this.days > 2) { result += (this.days - 2) * 1.5; } return result; case "new": result = this.days * 3; return result; case "childrens": result = 1.5; if (this.days > 3) { result += (this.days - 3) * 1.5; } return result; } return result; } 

customer .

statement.es6…

 function statement(customerArg, movies) { const customer = new Customer(customerArg, movies); let result = `Rental Record for ${customer.name}\n`; for (let r of customer.rentals) { result += `\t${r.movie.title}\t${r.amount}\n`; } result += `Amount owed is ${customer.amount}\n`; result += `You earned ${customer.frequentRenterPoints} frequent renter points\n`; return result; } 

class Customer...

  get frequentRenterPoints() { return this.rentals .map((r) => r.frequentRenterPoints) .reduce((a, b) => a + b) ; } get amount() { return this.rentals .reduce((total, r) => total + r.amount, 0); } 

rental customer , html- statement .

statement.es6…

  function htmlStatement(customerArg, movies) { const customer = new Customer(customerArg, movies); let result = `<h1>Rental Record for <em>${customer.name}</em></h1>\n`; result += "<table>\n"; for (let r of customer.rentals) { result += ` <tr><td>${r.movie.title}</td><td>${r.amount}</td></tr>\n`; } result += "</table>\n"; result += `<p>Amount owed is <em>${customer.amount}</em></p>\n`; result += `<p>You earned <em>${customer.frequentRenterPoints}</em> frequent renter points</p>\n`; return result; } 

ES2015 , , ( Java-). :

 function statement(customerArg, movies) { const customer = createCustomer(customerArg, movies); let result = `Rental Record for ${customer.name()}\n`; for (let r of customer.rentals()) { result += `\t${r.movie().title}\t${r.amount()}\n`; } result += `Amount owed is ${customer.amount()}\n`; result += `You earned ${customer.frequentRenterPoints()} frequent renter points\n`; return result; } function createCustomer(data, movies) { return { name: () => data.name, rentals: rentals, amount: amount, frequentRenterPoints: frequentRenterPoints }; function rentals() { return data.rentals.map(r => createRental(r, movies)); } function frequentRenterPoints() { return rentals() .map((r) => r.frequentRenterPoints()) .reduce((a, b) => a + b) ; } function amount() { return rentals() .reduce((total, r) => total + r.amount(), 0); } } function createRental(data, movies) { return { days: () => data.days, movieID: () => data.movieID, movie: movie, amount: amount, frequentRenterPoints: frequentRenterPoints }; function movie() { return movies[data.movieID]; } function amount() { let result = 0; switch (movie().code) { case "regular": result = 2; if (data.days > 2) { result += (data.days - 2) * 1.5; } return result; case "new": result = data.days * 3; return result; case "childrens": result = 1.5; if (data.days > 3) { result += (data.days - 3) * 1.5; } return result; } return result; } function frequentRenterPoints() { return (movie().code === "new" && data.days > 2) ? 2 : 1; } 

Function As Object . - ( createCustomer createRental ) JavaScript () . - . , . , , . , — .

All of these approaches require that print functions statementcall other functions to calculate the data they need. This can be done differently: pass this data to the report printing function in the data structure itself. With this approach, the calculation functions are used to transform the data structure in customersuch a way that it will contain all the data needed by the print function.

In terms of refactoring, this is an example of the not yet written Split Phase refactoring, which Kent Beck described to me last summer. With this refactoring, I divide the calculations into two phases, which communicate with each other through an intermediate data structure. We start this refactoring with the introduction of an intermediate data structure.

  function statement(customer, movies) { const data = createStatementData(customer, movies); let result = `Rental Record for ${data.name}\n`; for (let r of data.rentals) { result += `\t${movieFor(r).title}\t${amountFor(r)}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; function createStatementData(customer, movies) { let result = Object.assign({}, customer); return result; } 

customer , , Object.assign . . , .

rental .

function statement…

  function createStatementData(customer, movies) { let result = Object.assign({}, customer); result.rentals = customer.rentals.map(r => createRentalData(r)); return result; function createRentalData(rental) { let result = Object.assign({}, rental); return result; } } 

, createRentalData createStatementData , createStatementData , .

, , .

 function statement(customer, movies) { const data = createStatementData(customer, movies); let result = `Rental Record for ${data.name}\n`; for (let r of data.rentals) { result += `\t${r.title}\t${amountFor(r)}\n`; } result += `Amount owed is ${totalAmount()}\n`; result += `You earned ${totalFrequentRenterPoints()} frequent renter points\n`; return result; //… function createStatementData(customer, movies) { // … function createRentalData(rental) { let result = Object.assign({}, rental); result.title = movieFor(rental).title; return result; } } 

.

 function statement(customer, movies) { const data = createStatementData(customer, movies); let result = `Rental Record for ${data.name}\n`; for (let r of data.rentals) { result += `\t${r.title}\t${r.amount}\n`; } result += `Amount owed is ${data.totalAmount}\n`; result += `You earned ${data.totalFrequentRenterPoints} frequent renter points\n`; return result; function createStatementData(customer, movies) { let result = Object.assign({}, customer); result.rentals = customer.rentals.map(r => createRentalData(r)); result.totalAmount = totalAmount(); result.totalFrequentRenterPoints = totalFrequentRenterPoints(); return result; function createRentalData(rental) { let result = Object.assign({}, rental); result.title = movieFor(rental).title; result.amount = amountFor(rental); return result; } } 

, , , statement . createStatementData .

 function statement (customer, movies) { // body … function createStatementData (customer, movies) { // body … function createRentalData(rental) { … } function totalFrequentRenterPoints() { … } function totalAmount() { … } function movieFor(rental) { … } function amountFor(rental) { … } function frequentRenterPointsFor(rental) { … } } } 

createStatementData statement .

 function statement (customer, movies) { … } function createStatementData (customer, movies) { function createRentalData(rental) { … } function totalFrequentRenterPoints() { … } function totalAmount() { … } function movieFor(rental) { … } function amountFor(rental) { … } function frequentRenterPointsFor(rental) { … } } 

, HTML- statement , .

 function htmlStatement(customer, movies) { const data = createStatementData(customer, movies); let result = `<h1>Rental Record for <em>${data.name}</em></h1>\n`; result += "<table>\n"; for (let r of data.rentals) { result += ` <tr><td>${r.title}</td><td>${r.amount}</td></tr>\n`; } result += "</table>\n"; result += `<p>Amount owed is <em>${data.totalAmount}</em></p>\n`; result += `<p>You earned <em>${data.totalFrequentRenterPoints}</em> frequent renter points</p>\n`; return result; } 

createStatementData , () .

, . , . , HTML- . , . .

---

top-level-functions

  function htmlStatement(customer, movies) function textStatement(customer, movies) function totalAmount(customer, movies) function totalFrequentRenterPoints(customer, movies) function amountFor(rental, movies) function frequentRenterPointsFor(rental, movies) function movieFor(rental, movies) 

→

---

parameter-dispatch

  function statement(customer, movies, format) function htmlStatement() function textStatement() function totalAmount() function totalFrequentRenterPoints() function amountFor(rental) function frequentRenterPointsFor(rental) function movieFor(rental) 

→

---

classes

,

  function textStatement(customer, movies) function htmlStatement(customer, movies) class Customer get amount() get frequentRenterPoints() get rentals() class Rental get amount() get frequentRenterPoints() get movie() 

→

---

transform

,

  function statement(customer, movies) function htmlStatement(customer, movies) function createStatementData(customer, movies) function createRentalData() function totalAmount() function totalFrequentRenterPoints() function amountFor(rental) function frequentRenterPointsFor(rental) function movieFor(rental) 

→

---

I will begin with an example of top-level functions in order to select the conceptually simplest alternative as a basis for comparison. [2] It is simple because it divides the work into a number of pure functions, and everyone can access them from anywhere in the code. This is easy to use and easy to test - I can easily test any single function either using test cases or using REPL.

The negative side top-level-functions- a large number of repetitive transmission parameters. Each function should be given a data structure with movies, and level functionscustomer — . , . , , . — . , , , , .

. , . , , , , . , , — .

, , . parameter-dispatchdoes this by capturing the context in the top-level parameter list, which is then available as a general context for all nested functions. This works especially well with the original code, making refactoring from a single function to nested functions easier than in a language that does not have nested functions.

But the approach parameter-dispatchbegins to wobble when other general behaviors are required from my context, like an HTML response. I had to write something like a dispatcher to determine which function I want to call. Defining a format for a renderer is not very bad, but such a controller logic clearly smells bad. However, I wrote it, it still essentially copies the basic ability of a language to call a named function. And I'm heading for the path that quickly leads me to this nonsense:

 function executeFunction (name, args) { const dispatchTable = { //... 

, . . statement …

 const someValue = statement(customer, movieList, 'text'); 

… .

— . — . API , , , textStatement htmlStatement . .

, , ? - - , . , — . [3] This leads me to an example with classes in which you can capture the general context of users and movies in objects customerand rental. I set the context once, when I assert the objects, and then all further logic can use the general context.

Object methods are similar to local functions with partial application in the first example, except for the general context provided here by the constructor. Therefore, I am writing only local functions, not top-level functions. The caller specifies the context with a constructor, and then calls the local function directly. I can imagine local methods as a variant of partial application of abstract functions of the upper level on the general context of an object instance.

The use of classes introduces an additional concept - the separation of rendering logic from the calculation logic. One of the drawbacks of the original single function is that it mixes them together. The division into functions to some extent divides them, but they still exist in a single conceptual space. This is slightly wrong, I could put the computational functions in one file, and the rendering functions in another, link them with the corresponding import statements. But it seems to me that the general context in itself gives a hint how to group the logic into modules.

, . , . , , , — Uniform Access Principle . . transform , , . customer rental , transform createStatementData createRentalData . List And Hash . create…Data , .

transform , . transform , . , . . transform , . . - , / .

So, four approaches - which one to prefer? I do not want to write controller logic, so I would not use the approach parameter-dispatch. One could choose top-level-functions, but they quickly spoil the impression of themselves when the overall context increases in size. Even with just two arguments, I'd rather have built in functions to achieve other alternatives. It is harder to choose between classes and transformation; both approaches make it possible to make the overall context and the separation explicit. I’m not interested in cockfights, so I’ll just throw a coin and have the winner choose the lot.

Further refactoring

. — , , .

, . amount frequentRenterPoint . , , , . , .

, , , . — Java, . JavaScript . , , — . ( JavaScript , ). , , . , , . , , .

Notes

<sup>[1] - , «» // . JavaScript «», .

[2] parameter-dispatch , . , top-level-functions .

[3] «» .</sup>