0

I'm trying to implement the Builder Pattern to generate a JSON string of options that are passed into a library to generate widgets. I can't understand why at console.log below that this.options is undefined.

let Options = function(options) {
  this.options = options;
}

let OptionsObjectBuilder = function () {

  let options;

  return {
      addConstantLineToValueAxis: function (lineValue) {
          console.log(this.options); // EQUALS UNDEFINED SO CAN'T ADD TO THIS OBJECT
          this.options.valueAxis.constantLine.value = lineValue;
          return this;
      },
      build: function () {
          return new Options(this.options);
      }
  };
};

let option = new OptionsObjectBuilder().addConstantLineToValueAxis(1000000000).build();
SweetTomato
  • 509
  • 1
  • 6
  • 16
  • 1
    `this.options` does not refer to the `let options` declarations - they are two completely different things. – VLAZ Nov 05 '19 at 18:20
  • So this example is incorrect? http://zetcode.com/javascript/builderpattern/ – SweetTomato Nov 05 '19 at 18:43
  • So in the example in my last comment.. they are setting name with `this.name = ` but should by setting it with `name =` correct? – SweetTomato Nov 05 '19 at 19:14
  • Yeah, that example is misleading. The `this` keyword has different semantics than OOP languages. In short, you can expect `this` to refer to some object (how that's determined is the trickier part in JS) while the `let`/`var`/`const` variables *aren't* part of that object. The variables defined in the example are not actually used, each of the `setX` methods just attaches a property with a similar name to `this`. The `build()` method then will fail because it will use the variables from the top, which are never changed. – VLAZ Nov 05 '19 at 20:03
  • As for whether or not you need to use `name` or `this.name` - up to you, really. Both will work and both are consistent with the Builder pattern. A slightly more OOP angle would be to just attach to `this` and only work with properties but using the variables is also fine. You can implement the builder pattern a number of ways in JS and they are all valid. The key points are chaining and producing a new object at the end, so you do `new SomethingBuilder().setX(x).setY(y).setZ(z).build()` and get a `something` out of it. The implementation underneath can vary. – VLAZ Nov 05 '19 at 20:07

2 Answers2

1

There are two different ways for the builder to store the temporary states:

  • In the builder object itself (by setting this.options =)
  • In a closure (by setting options =)

The closure example has the benefit that the temporary builder state is not accessible to the outside.

You can use either way, as long as the builder uses them from the correct place. I will fix the broken example from the post you mentioned. I think they started using closures, and it didn't work because the param name was shadowing the closure variable and they ended up getting confused switching to using this instead. They forgot to update their build() function to read from the correct place.

Using builder object state - Exposes internal state

let Task = function(name, description, finished, dueDate) {

    this.name = name;
    this.description = description;
    this.finished = finished;
    this.dueDate = dueDate;
}

let TaskBuilder = function () {
    return {
        setName: function (name) {
            this.name = name;
            return this;
        },
        setDescription: function (description) {
            this.description = description;
            return this;
        },
        setFinished: function (finished) {
            this.finished = finished;
            return this;
        },
        setDueDate: function (dueDate) {
            this.dueDate = dueDate;
            return this;
        },
        build: function () {
            return new Task(this.name, this.description, this.isFinished, this.dueDate);
        }
    };
};

let builder = new TaskBuilder().setName('Task A').setDescription('finish book')
    .setDueDate(new Date(2019, 5, 12));
let task = builder.build();
// Notice the builder does expose the name/description... properties
console.log({builder, task});

Using closure variables - Hides internal state

let Task = function(name, description, finished, dueDate) {

    this.name = name;
    this.description = description;
    this.finished = finished;
    this.dueDate = dueDate;
}

let TaskBuilder = function () {

    let name;
    let description;
    let isFinished = false;
    let dueDate;

    return {
        setName: function (pName) {
            name = pName;
            return this;
        },
        setDescription: function (pDescription) {
            description = pDescription;
            return this;
        },
        setFinished: function (pFinished) {
            finished = pFinished;
            return this;
        },
        setDueDate: function (pDueDate) {
            dueDate = pDueDate;
            return this;
        },
        build: function () {
            return new Task(name, description, isFinished, dueDate);
        }
    };
};

let builder = new TaskBuilder().setName('Task A').setDescription('finish book')
    .setDueDate(new Date(2019, 5, 12));
let task = builder.build();
// Can't see the name/description... properties on the builder, just the methods
console.log({builder, task});
Ruan Mendes
  • 90,375
  • 31
  • 153
  • 217
0

I believe I should only ever use this when returning at the end of the add functions in the builder pattern. I'm still not sure why in the example (zetcode.com/javascript/builderpattern) I was basing my code off of.. they set values with this.name, but passed name in their build function.

// @Steven de Salas: expando function: https://stackoverflow.com/a/44014709/1432612
function buildObjectDepth(obj, base) {
    return Object.keys(obj)
      .reduce((clone, key) => {
        key.split('.').reduce((innerObj, innerKey, i, arr) => 
          innerObj[innerKey] = (i+1 === arr.length) ? obj[key] : innerObj[innerKey] || {}, clone)
        return clone;
    }, Object.assign({}, base));
}

let Options = function(options) {
  this.options = options;
}

let OptionsObjectBuilder = function () {

  let options = {};

  return {
      addConstantLineToValueAxis: function (lineValue) {
         options = buildObjectDepth({"valueAxis.constantLine.value": lineValue}, options);
         return this;
      },
      build: function () {
          return new Options(options);
      }
  };
};

let obj = new OptionsObjectBuilder().addConstantLineToValueAxis(1000000000).build();

str = JSON.stringify(obj);
str = JSON.stringify(obj, null, 4); // indented output.
alert(str);
SweetTomato
  • 509
  • 1
  • 6
  • 16
  • 1
    Their example is wrong . It doesn't call `build()`. If you run it yourself and call `build()` you will notice that the created task doesn't have the correct attributes, they are empty because the closure variables have never been set – Ruan Mendes Nov 05 '19 at 20:11