2
\$\begingroup\$

Today, a solution written in Python was posted to this challenge, and since I was inspired by it, I made a version in EcmaScript 6.

I tried to explore some new language constructs:

  • arrow functions
  • template Strings
  • let
  • for..of
  • generators

The code:

function* bottlesOfBeer(howMany=99){
  let pluralizeBottles = count => count !== 1 ? "bottles" : "bottle";
  let takeNext = next => next >= 1 ? next.toString() : "No more";
  for (let i = howMany; i >= 0; i--) {
    let bottle = pluralizeBottles(i);
    let bottleNext = pluralizeBottles(i-1);
    let left = takeNext(i);
    let toTake = i > 1 ? "one" : (i == 1 ? "it" : "no more");
    let toTakeNext = takeNext(i-1);
    let line1 = `${left} ${bottle} of beer on the wall, ${left.toLowerCase()} ${bottle} of beer.`;
    let line2 = `\nTake ${toTake} down and pass it around, ${toTakeNext.toLowerCase()} ${bottleNext} of beer on the wall.\n`;
    yield `${line1}${i > 0 ? line2 : ""}`;
  }
  yield `Go to the store and buy some more, ${howMany} bottles of beer on the wall.`;
}

for (let it of bottlesOfBeer()) {
  console.log(it);
}

This code can be copy/pasted into here to see it working: http://6to5.org/repl/

Did I use these constructs appropriately? I'd like to know if I somehow abused them.

And could it be made more readable?

What do you think, overall?

\$\endgroup\$
1
\$\begingroup\$

Style

There's a few inconsistent stylistic choices, mostly regarding whitespace.

 howMany=99                        // no spaces around "="
 let bottle = pluralizeBottles(i); // spaces around "="

Let

There's nothing wrong with the way you use let, but when talking about let you usually also talk about const since they're available since the same ES version.

You never mutate any of the variables you declare and initialize with let, which means they can all be changed to const.

// old
let bottle = pluralizeBottles(i);

// new
const bottle = pluralizeBottle(i);

This is also true for the for loop. By using const, you ensure that you never do an assignment to it.

// old
for (let it of bottlesOfBeer()) {

// new
for (const it of bottlesOfBeer()) {

Helper functions

Do not put your helper functions inside the generator. They will be created each time the generator is called -- there's no need for this. Move them outside. If you want to make them private, use modules (since we're talking about ES6), and just don't export them.

I'd also create more util functions. For example, the following line is a bit too long and complex. It's more complex than your pluralizeBottles function, so there's no reason why this shouldn't be extracted as well.

let toTake = i > 1 ? "one" : (i == 1 ? "it" : "no more");

Usage of generators

Your usage of generators is good. This is the type of problem they're created for.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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