tmerrick
tmerrick

Reputation: 19

Is there a way to refactor this Javascript for loop?

This is a test project I'm using to learn Javascript in. I have a for loop with if statements after that, that go into innerHTML's. Here is a for loop below. I'm wondering if how I can do this with say map()? I've looked online on how to use maps() but nothing shows stuff that's a little more complex then beginner understanding...which I'm a beginner.

for (let i = 0; i < data.length; i++) {
        let dataTitle = data[i].title;
        let dataTimeFrames = data[i].timeframes.daily;

        if (dataTitle === 'Work') {
            hours_work.innerHTML = `<p>${dataTimeFrames.current}hrs</p>`;
            past_work.innerHTML = `<p>Yesterday - ${dataTimeFrames.previous}hrs</p>`;
        }
        if (dataTitle === 'Study') {
            hours_play.innerHTML = `<p>${dataTimeFrames.current}hrs</p>`;
            past_play.innerHTML = `<p>Yesterday - ${dataTimeFrames.previous}hrs</p>`;
        }
        if (dataTitle === 'Play') {
            hours_study.innerHTML = `<p>${dataTimeFrames.current}hrs</p>`;
            past_study.innerHTML = `<p>Yesterday - ${dataTimeFrames.previous}hrs</p>`;
        }
        if (dataTitle === 'Exercise') {
            hours_exercise.innerHTML = `<p>${dataTimeFrames.current}hrs</p>`;
            past_exercise.innerHTML = `<p>Yesterday - ${dataTimeFrames.previous}hrs</p>`;
        }
        if (dataTitle === 'Social') {
            hours_social.innerHTML = `<p>${dataTimeFrames.current}hrs</p>`;
            past_social.innerHTML = `<p>Yesterday - ${dataTimeFrames.previous}hrs</p>`;
        }
        if (dataTitle === 'Self Care') {
            hours_care.innerHTML = `<p>${dataTimeFrames.current}hrs</p>`;
            past_care.innerHTML = `<p>Yesterday - ${dataTimeFrames.previous}hrs</p>`;
        }
    }

This code works just fine, I just want to learn how to refactor something like this.

Here is the repo for further understanding if you want: https://github.com/tmerrick17/time-tracking-dashboard

Upvotes: 0

Views: 99

Answers (3)

Louys Patrice Bessette
Louys Patrice Bessette

Reputation: 33933

Using some data-* instead of ids would simplify your task a lot.

Given this HTML sample (found on your GitHub):

<!-- Social Card -->
<div class="card-social">
  <div class="card-top card-img">
    <img src="public/images/icon-social.svg" alt="book icon">
  </div>
  <a class="card-bot-anchor" href="#">
    <div class="card-bot card__info">
      <h2>Social</h2>
      <img src="public/images/icon-ellipsis.svg" alt="3 dots icon">
      <p id="hours-social" data-title="Self Care" data-timeframe="current" class="hours-current margin-top1">hrs</p>
      <p id="past-social" data-title="Self Care" data-timeframe="previous"  class="past-toggle">Category Toggle</p>
    </div>
  </a>
</div>

Notice the addons

data-title="Self Care" data-timeframe="current"

Your loop would be

data.forEach(activity => {
  let r = activity.title,
    t = activity.timeframes.daily;

  document.querySelector(`[data-title='${r}'][data-timeframe='current']`).innerHTML = `<p class="hours-current">${t.current}hrs</p>`);
  document.querySelector(`[data-title='${r}'][data-timeframe='previous']`).innerHTML = `<p>Yesterday - ${t.previous}hrs</p>`;
}

And most likely you can discard the ids and the bunch of variables created for them.

Upvotes: 1

Joshua Wood
Joshua Wood

Reputation: 535

So, the first thing I would do is instead of having your elements already determined and then setting their html, you can have a function that grabs the right element based on the title, so I imagine you have something like this:

var hours_care = document.getElementById("hours_care")

instead, have a function that grabs the right elements based off the title. Something like this would be good:

function getHoursAndPast (title) {
  var lowerTitle = title.toLowerCase();
  var hours = document.getElementById("hours_" + lowerTitle)
  var past = document.getElementById("past_" + lowerTitle)
  return [hours, past]
}

This function returns an array of the elements you're trying to get. You can then take that and DE-structure this array inside your loop. I wouldn't use map, because you're not interested in getting an array back from this. Instead, use forEach

data.forEach(datum => {
  let dataTitle = datum.title;
  let dataTimeFrames = datum.timeframes.daily;
  const [hours, past] = getHoursAndPast(dataTitle);
  hours.innerHTML = `<p>${dataTimeFrames.current}hrs</p>`;
  past.innerHTML = `<p>Yesterday - ${dataTimeFrames.previous}hrs</p>`;
})

Upvotes: 1

Ryan O&#39;D
Ryan O&#39;D

Reputation: 350

One approach is to create an object that holds the references to the HTML elements. This breaks up the data specific information from the loop making it easier to manage. Of course, you're still left dealing with the somewhat lengthy dataObj, which isn't ideal.

const dataObj = {
  Work: {
    hoursElement: document.getElementById('hours_work'),
    pastElement: document.getElementById('past_work'),
  },
  Study: {
    hoursElement: document.getElementById('hours_study'),
    pastElement: document.getElementById('past_study'),
  }
  // Etc....
}

for (let i = 0; i < data.length; i++) {
  let dataTitle = data[i].title;
  let dataTimeFrames = data[i].timeframes.daily;

  dataObj[dataTitle].hoursElement.innerHTML = `<p>${dataTimeFrames.current}hrs</p>`;
  dataObj[dataTitle].pastElement.innerHTML = `<p>Yesterday - ${dataTimeFrames.previous}hrs</p>`;
}

Upvotes: 1

Related Questions