4
\$\begingroup\$

Adding Dynamic Slides to a slider used by a client shifts its index, need to account for that so i wrote this simple code, but i think it's pretty lame, is there any way to improve it and accomplish the same output?

var slides = {
  1: {
    content: "",
    group: 1
  },
  2: {
    group: 1
  },
  3: {
    content: "",
    group: 1
  },
  4: {
    content: "",
    group: 2
  },
  5: {
    content: "",
    group: 3
  },
  6: {
    content: "",
    group: 3
  },
  7: {
    content: "",
    group: 3
  },
  8: {
    content: "",
    group: 4
  },
  9: {
    content: "",
    group: 4
  },
  10: {
    content: "",
    group: 4
  },
  11: {
    content: "",
    group: 5
  },
  12: {
    content: "",
    group: 5
  }
};

var shiftCount = [],
  shiftCounter = 0,
  lastgroup, lastcount;
for (var slideIndex in slides) {
  var group = parseInt(slides[slideIndex].group, 10);
  if (group === 1) { //no shift caused by 1st group
    shiftCount.push(0);
  } else {
    if (group !== lastgroup) { //stay on current group.
      lastcount = shiftCount.length;
      lastgroup = group;
      shiftCount.push(shiftCount.length);
    } else { //fill rest of array with repeated count and match length.
      shiftCount.push(lastcount);
    }
  }
}
console.log(shiftCount);

Thanks!

\$\endgroup\$
1
\$\begingroup\$

You never use the shiftcounter variable, so feel free to remove it.

You don't care about the keys in the object you're iterating over, you only care about the objects at each key - so, it would be more appropriate to iterate just over the values, rather than the keys. Use Object.values instead of for..in.

The .group property is already an integer in the dataset, so there's no need to call parseInt with it. Since you want to extract the group property into a variable named group, you can do this more concisely with destructuring.

Rather than hard-coding the action to take when the group is 1 (the first), you can initialize lastGroup to 1 so that the //fill rest of array with repeated count and match length section will take care of the logic you need - it'll be more DRY.

const shiftCount = [];
let lastgroup = 1, // initial group
  lastcount = 0; // initial count
for (const { group } of Object.values(slides)) {
  if (group !== lastgroup) {
    lastcount = shiftCount.length;
    lastgroup = group;
    shiftCount.push(lastcount);
  } else { //fill rest of array with repeated count and match length.
    shiftCount.push(lastcount);
  }
}

const slides = {
  1: {
    content: "",
    group: 1
  },
  2: {
    group: 1
  },
  3: {
    content: "",
    group: 1
  },
  4: {
    content: "",
    group: 2
  },
  5: {
    content: "",
    group: 3
  },
  6: {
    content: "",
    group: 3
  },
  7: {
    content: "",
    group: 3
  },
  8: {
    content: "",
    group: 4
  },
  9: {
    content: "",
    group: 4
  },
  10: {
    content: "",
    group: 4
  },
  11: {
    content: "",
    group: 5
  },
  12: {
    content: "",
    group: 5
  }
};

const shiftCount = [];
let lastgroup = 1,
  lastcount = 0;
for (const { group } of Object.values(slides)) {
  if (group !== lastgroup) {
    lastcount = shiftCount.length;
    lastgroup = group;
    shiftCount.push(lastcount);
  } else { //fill rest of array with repeated count and match length.
    shiftCount.push(lastcount);
  }
}
console.log(shiftCount);

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.