ryanmcdermott / clean-code-javascript

:bathtub: Clean Code concepts adapted for JavaScript
MIT License
91.72k stars 12.3k forks source link

Clean code vs runtime data checking #372

Open pathvine opened 2 years ago

pathvine commented 2 years ago

Bad:

function combine(val1, val2) {
  if (
    (typeof val1 === "number" && typeof val2 === "number") ||
    (typeof val1 === "string" && typeof val2 === "string")
  ) {
    return val1 + val2;
  }

  throw new Error("Must be of type String or Number");
}

Good:

function combine(val1, val2) {
  return val1 + val2;
}

The example does promote clean code. However, this example has discarded the important runtime type checking and necessary validations and seems incomplete. I agree that we should have "combine" function to do just the "combine", but if other team members get overly confident that the combine function would perform validation/data type checking as well, wouldn't this promote confusion/bugs instead? Appreciate your insight on this.

Willaiem commented 2 years ago

You can possibly create an assertion:

const assertsValidCombineArgs = (val1, val2) => {
    const areArgsString = typeof val1 === "string" && typeof val2 === "string"
    const areArgsNumber = typeof val1 === "number" && typeof val2 === "number"

    if (!areArgsString || !areArgsNumber ) {
       throw new Error("Combine args must be of type String or Number");
    }
}

function combine(val1, val2) {
  assertsValidCombineArgs(val1, val2)
  // now we are sure, that val1 and val2 are either string or number

  return val1 + val2;
}

Fact, it looks like an oversight on the author's part.