4

So I want to find all items that match a string and replace them. For some reason, I can't get it to iterate and replace all. Any help would be appreciated

const word = ['h', 'e', 'l', 'l', 'o'];
var placeholder = word.map(e => {
  return "*"
});

let letter = 'l';

word.forEach(e => {
  if (e === letter) {
    placeholder.splice(word.indexOf(letter), 1, letter);
  }
});


console.log(placeholder);
Phil
  • 157,677
  • 23
  • 242
  • 245
Ryan Zeelie
  • 1,320
  • 5
  • 12
  • Just to clarify, do you want the result to be `**ll*`? – Phil May 07 '21 at 01:53
  • since you are doing a letter by letter replacement in the loop, splice seems like the wrong tool for this job – chiliNUT May 07 '21 at 01:55
  • Why not just sort out the replacements when you create the `placeholder`, ie `var placeholder = word.map(e => e === letter ? e : "*")`? – Phil May 07 '21 at 01:55
  • Yea the output should be `**ll*` . I should have explained a bit more. It's a hangman-type game. So I have an event listener setup. When a letter is selected, it'll check through the word then show the correct letters. I'll try out the map method. Thanks. Still don't see why it wouldn't work though since the forEach is iterating each element in the original array. – Ryan Zeelie May 07 '21 at 02:00
  • 2
    Because you are replacing the same index twice. Check what the value of `word.indexOf(letter)` returns each time and you will see it is the same each occurence – charlietfl May 07 '21 at 02:03

2 Answers2

2

Use the index parameter in the callback to forEach instead of always using the first index at which the letter is found.

const word = ['h', 'e', 'l', 'l', 'o'];
var placeholder = word.map(e => {
  return "*"
});

let letter = 'l';

word.forEach((e,i) => {
  if (e === letter) {
    placeholder.splice(i, 1, letter);
  }
});


console.log(placeholder);

Directly using map is much simpler.

const word = ['h', 'e', 'l', 'l', 'o'];
let letter = 'l';
const res = word.map(e => letter === e ? letter: "*");
console.log(res);
Unmitigated
  • 76,500
  • 11
  • 62
  • 80
  • [Using `splice` in a loop doesn't work](https://stackoverflow.com/questions/9882284/looping-through-array-and-removing-items-without-breaking-for-loop). – Bergi Jan 26 '22 at 19:33
  • @Bergi The first snippet does not modify the array it is iterating over. It only replaces elements of another array so the indexes do not shift. Try running it. – Unmitigated Jan 26 '22 at 19:37
  • Oh I missed that it doesn't change the length of the array (wouldn't have mattered which one), but actually replaced the element only. This really should be written as `placeholder[i] = letter;`, not using `splice`. – Bergi Jan 26 '22 at 19:41
0

Note that my answer isn't the most efficient... rather I wanted to explain why your function wasn't working and how you would have to edit it to make that approach work...

Your word.forEach function replaces the same letter in placeholder twice because Array.indexOf() always returns the index of the 1st occurrence... and you never update word.. you only update placeholder. You will need to make a copy of word and make changes to it as well...

const word = ['h', 'e', 'l', 'l', 'o'];
let tempWord = JSON.parse(JSON.stringify(word));
var placeholder = word.map(e => {
  return "*"
});

let letter = 'l';

tempWord.forEach(e => {
  if (e === letter) {
    placeholder.splice(tempWord.indexOf(letter), 1, letter);
    tempWord.splice(tempWord.indexOf(letter), 1, '*');    
  }
});


console.log(placeholder);
DynasticSponge
  • 1,416
  • 2
  • 9
  • 13