How Can I Refactor This?

1

Could anyone give me advice on how to refactor this chunk of code? The functions are very similar to each other, they only differ a little bit.

  function offline(streamer){
    let output = `
    <div class="col-xs-12 item">
        <div class="row center middle item-size">
          <div class="col-xs-4 avatar"><img src="https://maxcdn.icons8.com/Share/icon/Messaging//offline1600.png" alt="${streamer}"></div>
          <div class="col-xs-4 content">${streamer}</div>
          <div class="col-xs-4 del"><img class="icon-offline" src="https://images-na.ssl-images-amazon.com/images/I/719zM3RmAxL.png"></div>
        </div>
      </div>`;
    return output;
  }

  function online(streamer, logo, url, status){
    let output = `
         <div class="col-xs-12 item">
            <div class="row center middle item-size">
              <div class="col-xs-4 avatar"><a href="${url}" target="_blank"><img src="${logo}" alt="${streamer}"></a></div>
              <div class="col-xs-4 content"><a href="${url}" target="_blank"><em>${streamer}</em> - <span class="info">${status}</span></a></div>
              <div class="col-xs-4 del"><img class="icon-online" src="https://phpfoxexpert.com/products/forum/uploads/824_1.png"></div>
            </div>
          </div>`;
    return output;
  }
2

Why do you feel this needs to be refactored?

3

Since there are a lot of lines that are similar to each other. But I was only curious whether there’s a way to refactor it.

4

I don’t see too much repetition. You could get rid of the variables and just return the string templates. You could change these to be arrow functions. Other than that, I think it’s more important for these functions to be readable than anything else.

1 Like
5

The functions are short, do a single thing, and are easy to read. Good job with them, no need to refactor.

Maybe add some comments…

1 Like
6

I tend to use ES6 whenever I can. In this case, I could have chosen to either put the two functions into an object and do something like that:

app = {
   online(streamer, logo, url, status){
    let output = `
         <div class="col-xs-12 item">
            <div class="row center middle item-size">
              <div class="col-xs-4 avatar"><a href="${url}" target="_blank"><img src="${logo}" alt="${streamer}"></a></div>
              <div class="col-xs-4 content"><a href="${url}" target="_blank"><em>${streamer}</em> - <span class="info">${status}</span></a></div>
              <div class="col-xs-4 del"><img class="icon-online" src="https://phpfoxexpert.com/products/forum/uploads/824_1.png"></div>
            </div>
          </div>`;
    return output;
    }, the other function ...
}

or

 online = (streamer, logo, url, status) => {
    let output = `
         <div class="col-xs-12 item">
            <div class="row center middle item-size">
              <div class="col-xs-4 avatar"><a href="${url}" target="_blank"><img src="${logo}" alt="${streamer}"></a></div>
              <div class="col-xs-4 content"><a href="${url}" target="_blank"><em>${streamer}</em> - <span class="info">${status}</span></a></div>
              <div class="col-xs-4 del"><img class="icon-online"     src="https://phpfoxexpert.com/products/forum/uploads/824_1.png"></div>
                </div>
          </div>`;
return output;
}

But I think it’s easier to use function nameOfFunction() {code} when there are just two functions

8

:scream: Thank you for having taken your time and refactoring my code. :yum: I’ve only had a glance at it, but I’m going to look at it carefully. I believe I’ll learn some new stuff from it. Cheers!

Eidt: I probably made a mistake when I had decided to use the FlexBox grid library(CSS). Not only made it my CSS huge, but it also forced me to use so many classes, which resembled each other.