How would you refactor this JS function?

During the weekend, I found the following small JS function in a blog post:

const lineChecker = (line, isFirstLine) => {
  let document = ``;

  if (line !== "" && isFirstLine) {
    document += `<h1>${line}</h1>`;
  } else if (line !== "" && !isFirstLine) {
    document += `<p>${line}</p>`;
  } else if (line === "") {
    document += "<br />";
  }

  return document;

};

I refactored it and was thinking that it could be an excellent beginner-level refactoring kata.

How would you refactor it?

Comments

[jameslongstaff]

Personally I'd do..

const lineChecker = (line, isFirstLine) => {
  let document = "<br />";

  if (line.length) {
    const tag = isFirstLine ? 'h1' : 'p';
    document = `<${tag}>${line}</${tag}>`;
  }

  return document;
};

[tq-bit]

Leaving out all further context, this might be one of the rare occasions where to make use !! (double ! operator)

const lineChecker = (line = '', isFirstLine = false) => {
  if (!!line) {
    return isFirstLine ? `<h1>${line}</h1>` : `<p>${line}</p>`;
  }
  return '<br/>';
};

Note: After looking through some other solutions, I figured using !! introduces (or solves? I wouldn't know) a problem when passing unexpected falsy - values such as null, false or undefined.

I'd still say it's a pretty good example for refactoring. Even though it seems fairly easy to overengineer if you adhere to the 'functions do a single thing' mantra.

[SenthilKumaranC]

I have refactored like below. Divided all conditions to Spec Units and Grouped Spec Units. Check my Github Link to see my code.

const specApis = specificationGroup();

specApis.addSpecObject(headerSpecGroup, headerSpecGroupCallback);
specApis.addSpecObject(paragraphSpecGroup, paragraphSpecGroupCallback);
specApis.addSpecObject(isEmpty, breakSpecGroupCallback);

export const lineChecker = (line, isFirstLine) => {
    let initialDocument = "";
    return specApis.validateSpecs({ initialDocument, line, isFirstLine });
}

github.com/SenthilKumaranC/How-To-...

[npc pratt]

One-liner:

const lineChecker = (line, isFirstLine) => line ? isFirstLine ? `<h1>${line}</h1>` : `<p>${line}</p>` : "<br />"

[lucus-30]

functional implementation with ramda

import { always, curry, ifElse, isEmpty, pipe, split, join, map }  from "ramda"

const html = curry((tagName, str) => `<${tagName}>${str}</${tagName}>`)
const h1 = html("h1")
const p = html("p")
const br = always("<br>")

const formatLines = map(ifElse(isEmpty, br, p))
const formatText = ([title, ...lines]) => [h1(title), ...formatLines(lines)]

const textToHTML = pipe(
  split("\n"),
  formatText,
  join("\n")
)

[Derek Enos]

Here's my one-liner that includes a hilarious immediately-invoked arrow function:

const lineChecker = (line, isFirstLine) =>
      line === ""
      ? "<br />"
      : (tag => `<${tag}>${line}</${tag}>`)(isFirstLine ? "h1" : "p")

[Sebastien Rodrigues]

const lineChecker = (Line,isFirstLine) =>
return line === « » ? « 
 » :
isFirsLine ? « H1 … » :

;

[Chris Howard]

This is a simple as I could get it while retaining readability

const lineChecker = (line = "", isFirstLine = false ) => {
  let document = isFirstLine ? `<h1>${line}</h1>` : `<p>${line}</p>`
  return line ? document : `<br />`
}

[radotooo2]

const lineChecker = (line, isFirstLine) => {
  const document = ``;

  if (line === "") return document.concat("<br />");

  isFirstLine ? document.concat(`<h1>${line}</h1>`) : document.concat(`<p>${line}</p>`);

  return document;
};

[Ryan Kuruppu]

I'd have to see the rest of the code to decide whether or not this would be the best case but for now

const lineChecker = (line, isFirstLine) => {
    if(line !== "" && line != null)
        return isFirstLine ? `<h1>${line}</h1>` :  `<p>${line}</p>`;

    return "<br />";
}

Note that I've used != instead of !== in order to check for undefined. If you were going the strict approach you'd have used !== instead but because we're unaware of what will be coming in line. We're using a non-strict check here. I think this is a fairly simple solution in this case