Fix Object.assign example

[connec]

The Object.assign example is currently incorrect.

function createMenu(config) {
  Object.assign(config, {
    title: 'Foo',
    body: 'Bar',
    buttonText: 'Baz',
    cancellable: true
  });
}

createMenu({ title: 'Not Foo' });

The title property will always be set to 'Foo'!

I was going to make a PR with a fix, but the approach really depends on whether the mutation of config is desirable.

How I would do it (this does not behave identically to the "bad" example):

function createMenu(config) {
  // makes a copy of `config`, with default values
  config = Object.assign({
    title: 'Foo',
    body: 'Bar',
    buttonText: 'Baz',
    cancellable: true
  }, config);
}

How you could do it with mutation (this is identical to the "bad" example):

function createMenu(config) {
  // mutates `config`, setting default values
  Object.assign(config, Object.assign({
    title: 'Foo',
    body: 'Bar',
    buttonText: 'Baz',
    cancellable: true
  }, config));
}

[halfzebra]

Sorry if I'm overthinking it, but I find it rather confusing to provide the example of object mutation in a procedural style, while advising to favor functional programming over imperative programming.

[rkichenama]

If the purpose is to mutate config to add defaults but keep changes, then the example should be

function createMenu(config) {
  Object.assign(
    config, // object to merge changes into
    {
      title: 'Foo',
      body: 'Bar',
      buttonText: 'Baz',
      cancellable: true
    }, // defaults
    config // overrides
);
}

createMenu({ title: 'Not Foo' });

IMHO, I think this is the case since there is no return and we are dealing with objects, passed by reference.

[kwelch]

I would say the best option would be to maintain the passed in parameter and create a new object all together that use defaults then user param.

function createMenu(options) {
  let config = Object.assign({}, {
      title: 'Foo',
      body: 'Bar',
      buttonText: 'Baz',
      cancellable: true
    }, // defaults
    options // overrides
  );
}

[rkichenama]

Good have to return the be object `config`

…

On Thu, Jan 5, 2017, 15:43 Kyle Welch ***@***.***> wrote: I would say the best option would be to maintain the passed in parameter and create a new object all together that use defaults then user param. function createMenu(options) { let config = Object.assign({}, { title: 'Foo', body: 'Bar', buttonText: 'Baz', cancellable: true }, // defaults options // overrides ); } — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABjwmaSKZ3XshXMv1qIBTXxz5dE_Yfwdks5rPVXjgaJpZM4La-SD> .

[connec]

@rkichenama that example does not work because config has already mutated when the 'overrides' are applied:

let config = { key: 'override' }
Object.assign(config, { key: 'default' }, config)
console.log(config.key) // default

Hence the need for an additional Object.assign.

@kwelch that option is equivalent to the non-mutating version I specified - the defaults are already in a new object, so merging them into another new object is redundant.

---

Anyway, looks like this was fixed in d00ca4c with @kwelch's version.

[kwelch]

@connec I agree ours are the same, I just prefer not editing parameters if you are attempting to be functional.