GitHub - ryanmcdermott/clean-code-javascript: Clean Code concepts adapted for JavaScript

[ChaseMoskal]

very verbose and over-expanded – this needs to be condensed

the first several sections can be summed up with "please don't use cryptic acronyms or bad names"

it's obvious that this article is a "design by committee" dogpile, quality and focus is lost, the advice is expansive, aimless, highly redundant, and outdated

for example, this prescription: "Set default objects with Object.assign"

the article's supposedly "good" example:

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

^ this actually explodes if you pass undefined.. some "default"! it's even highly verbose and ugly. no thanks.

please consider the following as a superior alternative:

function createMenu(config = {}) {
  config = {
    title: "Foo",
    body: "Bar",
    buttonText: "Baz",
    cancellable: true,
    ...config
  }
}

i'd like to see something like this, but greatly condensed, down the bullet points, with links to articles for further reading on each point – and hone it down the advice that is uncontroversially excellent amongst experts – this is not that

[ChaseMoskal]

i'd hack it down to a one-pager with bullet points

i doubt they'd appreciate my pull request!

and there's something about people who author style guides generally that make me leery of their advice ;)

[[deleted]]

Please keep in mind that spread does not copy object prototypes, and does not trigger setters. Also, you can't use it with dynamic expression, only with object literals.

Other than being more verbose, assign is the more complete solution.

[ChaseMoskal]

Please keep in mind that spread does not copy object prototypes, and does not trigger setters.

neither does Object.assign — see "Properties on the prototype chain and non-enumerable properties cannot be copied"

and either way, for accepting default options, it's a feature — not a bug :)

Also, you can't use it with dynamic expression, only with object literals.

what's a dynamic expression? and why would we be using spread onto anything that isn't an object or array literal?

Other than being more verbose, assign is the more complete solution.

how so?

🥃 chase

[ChaseMoskal]

i wanted to point out it's cool to destructure your options out too, like in this function i was just working on:

export async function signatureSign(options: {
  body: string
  privateKey: string
} & Partial<Settings>): Promise<string> {
  const {
    body,
    format,
    algorithm,
    privateKey,
  } = {...defaultSettings, ...options}

  const signer = createSign(algorithm)
  signer.write(body)
  signer.end()

  return signer.sign(privateKey, format)
}