HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

99 Bottles of Beer using polymorphism

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
polymorphismusingbottlesbeer

Problem

This was part of an exercise from exercism.io. If you fancy, view the repo here.

Given problem statement:


Write a program which produces the lyrics to that beloved classic, that field-trip favorite: 99 Bottles of Beer on the Wall.

  • Remove as much duplication as you possibly can.



  • Optimize for readability, even if it means introducing duplication.



  • If you've removed all the duplication, do you have a lot of


conditionals? Try replacing the conditionals with polymorphism, if it
applies in this language. How readable is it?

Note: the spec was given as part of the problem.

I'm fairly inexperienced with javascript so I'm wondering if there's anything that can be improved.

I'm especially interested in any critiques of how I implemented polymorphism using the factory pattern.

beer-song.js

``
class BeerSong {
sing(...verses) {
this.validateArguments(verses);
return this.verses(...verses);
}

verses(starting, ending = 0) {
let verses = '';

while(starting >= ending) {
verses += this.verse(starting) + '\n';
starting--;
}

return verses.trim();
}

verse(number) {
const factory = new BottleFactory();
const bottle = factory.getBottle(number);
const nextBottle = factory.getBottle(bottle.successor());

return
+
${bottle.quantity().capitalize()} ${bottle.container()} of beer on the wall, +
${bottle.quantity()} ${bottle.container()} of beer.\n +
${bottle.action()}, +
${nextBottle.quantity()} ${nextBottle.container()} of beer on the wall.\n`;
}

validateArguments(...args) {
if (args.length > 2 || args.length < 1) {
throw "Invalid arguments: max 2.";
}
if (args.length == 2 && args[0] < args[1]) {
throw "Invalid arguments: must be in decreasing order.";
}
}
}

class Bottle {
constructor(number) {
this.number = number;
}

container() {
return 'bottles';
}

quantity() {
return this.number.toString();
}

action() {

Solution

Your approach:

-
Define duplicate objects that contain information about plural and singular forms and what to do next.

-
Access these from a single conditional (your switch / case) in an object-factory object.

Pros:

  • You have used polymorphism to replace conditionals.



Cons:

  • The scope and hierarchy of bottle objects, and how they apply to the song output, are a little confusing.



A possible alternative:

Your BottleFactory implementation is readable, and I must say innovative. However, the approach creates a lot of extra code that isn't exactly aiding readability or re-use.

You have a BottleFactory, with three bottle-like objects: Bottle, OneBottle and ZeroBottle.

It would be neater not to think of the bottles themselves as objects, but simply values of a counter property applied to a container object, the "wall" in this case. The "wall" object would replace your three bottle-like-objects and bottle-factory, and a single instance would serve for the whole song. Indeed, each verse of the song itself can be thought of as a message accessble to the user about the current status of the "wall", along with the next action to be undertaken.

Super-class constructor

You are creating a new instance of BottleFactory every time the verse method is called. You should move the line const factory = new BottleFactory(); to a constructor function for the BeerSong class, which will run only once.

Design Patterns

Remember the "Beer Song" exercise is supposed to prepare you for real examples. You will need to think more about which things should be objects and which should be their properties and methods.

The goal is usually to get your implementation as close as possible to a standard design pattern. In this case, your program might be one of a general type that displays count-downs and count-ups. Examples of this you can look at in Javascript might be counters (eg. word count or number of lines) in online forms, that are often handled on the client side and can be viewed and downloaded as examples.

Inspiration for a design pattern to use could also come from the song itself. Here, you get a gold star for recognizing the actions implied in the song, and using them as action() methods in your program. The song also inpires a strategy for finishing software projects after work hours (it's not a productive one).

Type-matching

Remove number from:

return new OneBottle(number);
        break;
      case 0:
        return new ZeroBottle(number);


as you have defined constructors for OneBottle and ZeroBottle that take no arguments.

While this works in Javascript, automated code review / debug tools will check whether arguments applied to a function match the expected type, and give you nasty messages about it. Which they do for very important reasons. Losing track of similar-looking function calls, and what kind of arguments they are supposed to take, will become a cause of problems down the line, especially in languages like Javascript, where mismatches such as extra arguments are handled silently.

BeerWall object

Here is my example of a wall object that handles bottles as subordinate objects.

The BeerWall object contains an array of bottle objects. It would be interfaced with via a song object.

Pros:

  • The actions stated in the output (the song) match exactly what is happening to the BeerWall object. (eg. when a bottle is "taken down" it is popped from the array.)



  • Special cases are defined as properties on bottle objects in the Bottles array, rather than making new sub-classes for them.



Cons:
  • I have the dumb song stuck in my head.



```
var plural = ["s", ""]
var short_status = "%n% bottle%p% of beer on the wall."
var long_status = "%n% bottle%p% of beer on the wall, %n% bottle%p% of beer."
var default_action_text = "Take one down and pass it around, "
var buy_text = "Go to the store and buy some more, "

class BeerWall {

constructor(n) {
this.fill = n
this.buy_bottles()
}

defs() {
this.default = {
action_text : default_action_text,
action : this.take_one_down,
plural : 0
}
this.bottles[0] = {
action_text : buy_text,
action : this.buy_bottles,
alt_id : "no more"
}
this.bottles[1] = {
plural : 1
}
}

buy_bottles() {
this.bottles = []
for (var i = 0; i <= this.fill; i++) {
this.bottles.push({})
}
this.defs()
return this.output(buy_text + short_status, this.fill)
}

take_one_down() {
var n = this.bottles.length - 1
var action_text = this.bottles[n].action_text || this.default.action_text
this.bottles.pop()
n -= 1
return this.output(action_text + short_status, n)
}

status() {
return this.output(long_status, this.bottles.length - 1)
}

action() {
var n = this.bottles

Code Snippets

return new OneBottle(number);
        break;
      case 0:
        return new ZeroBottle(number);
var plural = ["s", ""]
var short_status = "%n% bottle%p% of beer on the wall."
var long_status = "%n% bottle%p% of beer on the wall, %n% bottle%p% of beer."
var default_action_text = "Take one down and pass it around, "
var buy_text = "Go to the store and buy some more, "

class BeerWall {

    constructor(n) {
        this.fill = n
        this.buy_bottles()
    }

    defs() {
        this.default = {
            action_text : default_action_text,
            action : this.take_one_down,
            plural : 0
        }
        this.bottles[0] = {
            action_text : buy_text,
            action : this.buy_bottles,
            alt_id : "no more"
        }
        this.bottles[1] = {
            plural : 1
        }
    }

    buy_bottles() {
        this.bottles = []
        for (var i = 0; i <= this.fill; i++) {
            this.bottles.push({})
        }
        this.defs()
        return this.output(buy_text + short_status, this.fill)
    }

    take_one_down() {
        var n = this.bottles.length - 1
        var action_text = this.bottles[n].action_text || this.default.action_text
        this.bottles.pop()
        n -= 1
        return this.output(action_text + short_status, n)
    }

    status() {
        return this.output(long_status, this.bottles.length - 1)
    }

    action() {
        var n = this.bottles.length - 1
        var action = this.bottles[n].action || this.default.action
        return action.call(this)  
    }

    output(text,n) {
        return {
            text: text,
            n: this.bottles[n].alt_id || String(n),
            p: plural[this.bottles[n].plural || this.default.plural]
        }
    }
}

// Try it out!!
c = 10    // number of bottles to start with  
var mywall = new BeerWall(c)
for (var i = 0; i <= c; i++) {
    console.log(mywall.status())
    console.log(mywall.action())
}
// The above loop logs all lines for the song as objects of the form:
// {   n: (number of bottles)
//     p: ("s" or "" for plural)
//     text: (appropriate text with placeholders for n and p)   }
// The wall restocks itself and can continue forever.

Context

StackExchange Code Review Q#154210, answer score: 4

Revisions (0)

No revisions yet.