CODEWITHSUNDEEP
javascript
Brandon
Brandon

Reputation: 1195

Why isn't this function removing the vowels?

I created a function that takes a string converts the string to an array and then compares each string character to each vowel in an array, removing the string character if it matches one. It doesnt seems to be working correctly on longer words. For example with "tom" the o would be removed but with "johnson" only the first o is removed and then the n is removed at the end as well. I'm not seeing the issue.

function removeVolwels(string){

  let strAr= function(string){
     //converts to to lower case and turns string into an array
      let s= string.toLowerCase();
      let strAr= s.split("");
      return strAr;
  }(string);
   //stores vowels
  let vowels=["a","e","o","i","u","y"];

  //loops through each character
  for(let i=0; i < string.length -1; i++){
      console.log("i index " + i);
  //compares each  character in the string to a every vowel until it matches one
      for(let j=0; j < vowels.length; j++){
          console.log("j index " + j + " and " +vowels[j]);
         if(string[i] === vowels[j]){
              console.log(string[i].toString() + " === "+vowels[j]);
             console.log("removed " + string[i]);
            //removes vowel if letter matches one
             strAr.splice(i,1);
             console.log(strAr)
         } 
      }
  }
  
  return strAr.join("");
}

console.log('tom => ' + removeVolwels('tom'));
console.log('johnson => ' + removeVolwels('johnson'));

Upvotes: 0

Views: 186

Answers (4)

Tom O.
Tom O.

Reputation: 5941

Here's a simpler approach that utilizes Array.prototype.filter,Array.prototype.join, and Array.prototype.indexOf methods. The following code also uses String.prototype.split method to convert a string into a character array:

//1st param is the string you want to remove letters from
//2nd param is an array containing the letters to remove
function removeLetters(str, toRemove) {
  //Create an array - each index contains a single character
  let letters = str.split('');

  //Use Array.prototype.filter method to remove any letter that occurs in "toRemove" array
  let filtered = letters.filter(letter => toRemove.indexOf(letter) === -1)

  //Turn the filtered array back into a string
  return filtered.join('');
}

//The letters we want to remove are all vowels, so:
const vowels = ['a', 'e', 'i', 'o', 'u', 'y'];

//Some test cases
const word = "tommy";
const word2 = "johnson";
const word3 = "aeioux";
console.log(removeLetters(word, vowels));
console.log(removeLetters(word2, vowels));
console.log(removeLetters(word3, vowels));

Upvotes: 0

elwin013
elwin013

Reputation: 517

The problem is that you call strAr.splice(i,1);.

So, you have word "johnson", and after removing first "o" you've got "jhnson", but current length of string is 6 instead of 7 in the beginning!

When you get to next "o" - i have value of 5 (which is correct for "johnson" string but not "jhnson").

Also, there is another bug in loop - you have condition i < string.length -1. That means you will never reach last character. It should be i < string.length.

So, if you want to reuse your solution you can write something like this: 

function removeVolwels(string){

  let strAr= function(string){
     //converts to to lower case and turns string into an array
      let s= string.toLowerCase();
      let strAr= s.split("");
      return strAr;
  }(string);
   //stores vowels
  let vowels=["a","e","o","i","u","y"];
  let returnVal = [];
  //loops through each character
  for(let i=0; i < string.length; i++){
      console.log("i index " + i);
      // simple flag to match if letter should be added to return array
      let shouldBeAdded = true;
      //compares each  character in the string to a every vowel until it matches one
      for(let j=0; j < vowels.length; j++){
          console.log("j index " + j + " and " +vowels[j]);
          if(string[i] === vowels[j]){
              // when it is some of the vowels it should not be added, so we change the flag, and break 'vowel loop'
              shouldBeAdded = false;
              break;
          } 
      }
      // if flag is true then add letter to result array
      if(shouldBeAdded === true) {
          returnVal.push(string[i])
      }
  }
  
  return returnVal.join("");
}

console.log('tom => ' + removeVolwels('tom'));
console.log('johnson => ' + removeVolwels('johnson'));

Upvotes: 5

j08691
j08691

Reputation: 208040

You seem to be overcomplicating things a bit. Below is a much more streamlined way of doing what you're after (code commented).

function removeVowels(string) {
  // convert string to lowercase and split into array 's'
  let s = string.toLowerCase().split("");

  // define our list of vowels
  let vowels = ["a", "e", "o", "i", "u", "y"];

  // loop over array 's' in reverse. if the letter we're iterating over is in the vowels array, remove it. We do this in reverse because we'd skip letters if we went from front to back due to the splice.
  for (let i = s.length-1; i >= 0; i--) {
    if (vowels.indexOf(s[i]) > -1) {
      s.splice(i, 1); // 'i' is the index to start at (which we get from our loop) and 1 is the number of characters to remove.
    }
  }
  return s.join("");
}

console.log('tom => ' + removeVowels('tom'));
console.log('johnson => ' + removeVowels('johnson'));
console.log('aoeieyyozoyyeieoa => ' + removeVowels('aoeieyyozoyyeieoa'));

Upvotes: 2

Ankush Sharma
Ankush Sharma

Reputation: 677

Because after executing, 

strAr.splice(i,1)

index in original string and index in strAr are not same for the same character. So, you need to twist your logic for that.

Upvotes: 1

Related Questions