2

I'm hoping someone can explain to me why I need to use "toLowerCase()" if I'm already using a regular expression that is case insensitive "i". The exercise is a pangram that can accept numbers and non-ascii characters, but all letters of the alphabet MUST be present in lower case, upper case, or mixed. I wasn't able to solve this exercise correctly until I added "toLowerCase()". This is one of the javascript exercises from exercism.io. Below is my code:

var Pangram = function (sentence) {
  this.sentence = sentence;
};

Pangram.prototype.isPangram = function (){
  var alphabet = "abcdefghijklmnopqrstuvwxyz", mustHave = /^[a-z]+$/gi, 
  x = this.sentence.toLowerCase(), isItValid = mustHave.test(x);

  for (var i = 0; i < alphabet.length; i++){
    if (x.indexOf(alphabet[i]) === -1 && isItValid === false){
      return false;
    }

  }
  return true;

};

module.exports = Pangram;
Obsidian Age
  • 41,205
  • 10
  • 48
  • 71
Veronica
  • 45
  • 5

2 Answers2

2

The regex may not be doing what you think it's doing. Here is your code commented with what's going on:

Pangram.prototype.isPangram = function (){
  var alphabet = "abcdefghijklmnopqrstuvwxyz", mustHave = /^[a-z]+$/gi, 
  x = this.sentence.toLowerCase(), isItValid = mustHave.test(x);

  // for every letter in the alphabet
  for (var i = 0; i < alphabet.length; i++){
    // check the following conditions:
    // letter exists in the sentence (case sensitive)
    // AND sentence contains at least one letter between a-z (start to finish, case insensitive)
    if (x.indexOf(alphabet[i]) === -1 && isItValid === false){
      return false;
    }

  }
  return true;

}

The logic that is checking whether each letter is present has nothing to do with the regex, the two are serving separate purposes. In fact, based on your description of the problem, the regex will cause your solution to fail in some cases. For example, assume we have the string "abcdefghijklmnopqrstuvwxyz-". In that case your regex will test false even though this sentence should return true.

My advice would be to remove the regex, use toLowerCase on the sentence, and iterate through the alphabet checking if the sentence has each letter - which you seems to be the track you were on.

Below is a sample solution with some tests. Happy learning!

function isPangram (str) {
  const alphabet = 'abcdefghijklmnopqrstuvwxyz'
  const strChars = new Set(str.toLowerCase().split(''))

  return alphabet.split('').every(char => strChars.has(char))
}

const tests = [
  "abc",
  "abcdefghijklmnopqrstuvwxyz",
  "abcdefghijklmnopqRstuvwxyz",
  "abcdefghijklmnopqRstuvwxyz-",
]

tests.forEach(test => {
  console.log(test, isPangram(test))
})
Damon
  • 4,216
  • 2
  • 17
  • 27
  • thank you so much! I used the regEx to make sure I'm checking for a-z or A-Z. I will play around without using regEx. Thanks a lot for your sample solution. I hope I can one day write like that. – Veronica Dec 11 '17 at 04:13
  • You are right. I removed the regExp, and this still works. Thank you. I spent some time on this. Thank you so much! – Veronica Dec 11 '17 at 04:20
  • @Veronica glad I could help! I'll also point out that the regex in your code is only ever "executed" once - when you assign `isItValid = mustHave.test(x)`. After that point the variable `isItValid` is cached as `true` or `false` for the rest of the code. As you iterate, it is not testing value by value, but rather you are basically saying `if (someCondition && true)` or `if(someCondition && false)` - that is to say `isItValid` is constant throughout the iteration - which is another indication that the code should be restructured. – Damon Dec 11 '17 at 04:54
1

It's because you're manually checking for lowercase letters:

if (x.indexOf(alphabet[i]) === -1)

alphabet[i] will be one of your alphabet string, which you have defined as lowercase.

It looks like you don't need the regex at all here, or at least it's not doing what you think it's doing. Since your regex only allows for alpha characters, it will fail if your sentence has any spaces.

Andy Ray
  • 30,372
  • 14
  • 101
  • 138
  • yes. Thank you. I really thought the regExp was checking to make sure I was using a-z or/and A-Z; that's why I used "i" to make sure the sentence is case insensitive. I just removed it and still works. I've been going crazy! thank you! – Veronica Dec 11 '17 at 04:23
  • If this is the right answer, click the empty check mark next to it to mark it as correct to help others find the right answer :) Make sure to do this for all questions you have where there is a correct answer – Andy Ray Dec 11 '17 at 04:27