DEV Community

Discussion on: How would you refactor this JS function?

View post
Collapse
 
austinshelby profile image
Austin Shelby •

I would first start thinking about what this function actually does.

For me the name lineChecker is a little bit misleading because it looks like the function actually parses strings to html.

This is what I came up with:

const parseStringToHtml = (
  isFirstLine: boolean,
  line?: string
): string => {
  if (!line) {
    return "<br />";
  } else if (isFirstLine) {
    return `<h1>${line}</h1>`;
  } else {
    return `<p>${line}</p>`;
  }
};
Enter fullscreen mode Exit fullscreen mode

For a function this small there isn't really much to refactor. Parsing markdown has been done thousands of times already and there are libraries that do this perfectly.

Cool puzzle nonetheless.

Collapse
 
rkallan profile image
RRKallan • • Edited

Adding the types makes sense.

Only the else if else statement could be replaced. My suggestion based on your example

const parseStringToHtml = (line = "", isFirstLine = false): string => {
    if (!line) return "<br />";

    if (isFirstLine) return `<h1>${line}</h1>`;

    return `<p>${line}</p>`;
};
Enter fullscreen mode Exit fullscreen mode

edited

Collapse
 
siddharthshyniben profile image
Siddharth •

I'd suggest putting the line argument in the function first, as line is more important and isFirstLine can have a default value

Thread Thread
 
rkallan profile image
RRKallan •

true i just copy paste it and didn't read the arguments.

Collapse
 
jamesthomson profile image
James Thomson •

Extra bonus points for renaming the function. This would be my first improvement. Even if you left the function as is and just renamed it to describe its purpose that's a huge improvement in my book.

Collapse
 
lgrammel profile image
Lars Grammel • • Edited

The renaming idea + introducing types is great!

To me, the method seems to be more of a print than a parse method (but I don't have the full context, just saw it in isolation).