CODEWITHSUNDEEP
javascriptinfinite-loop
rfheise
rfheise

Reputation: 74

For Loop Creates An Infinite Loop When It Detects A Non-Alphabetical Character

During the for loop iteration i does not get incremented if a non-alphabetical character is detected in the if statement.

The nonnumeric function returns true if the character is non-alphabetical else it returns false. When I tested the code inside the for loop I had the console print out the value of i. Whenever the code reached a non-alphabetical character i would not get incremented and thus would create an infinite for loop. 

function isUpper(str){
  for(i = 0; i < str.length; i++){
    if(str.charAt(i) == str.charAt(i).toUpperCase()){
      if(!nonnumeric(str.charAt(i))){
        return true;
      };
    };
  };
  return false;
};

I expect it to return true if there is an uppercase character inside the string. If there is not an uppercase character in the string I expect it to return false.

Upvotes: 0

Views: 121

Answers (2)

paoiherpoais
paoiherpoais

Reputation: 334

Couple things that I believe are causing the issue:

  1. You're looking through each character in the string individually, then returning either true or false. If your goal is to determine whether the string contains at least one uppercase letter, then you first need to iterate through the entire string. Right now, if your first character isn't uppercase it won't return true and so will instantly return false.
  2. When you iterate through the string you'll hit non-alpha characters like spaces and punctuation. If you test whether a "lower case" period equals an "upper case" period, it'll evaluate to true, which could throw a false positive.

Solution: I've changed the function a little bit. Now, it iterates through the entire string and it uses a Regular Expression ("Regex") to determine if the value being tested is indeed a letter. If it runs into a letter that's uppercase, the count is increased, allowing the code to go through the entire string. We use the logical "&&" operator to accomplish two logical checks inside of our final if function.

How to improve: You could make this code better by returning true as soon as the count is increased above 0, but I'll leave that up to you if you want.

EDIT: Moved the 2nd If statement into the for loop so that as soon as the condition is met it returns true, rather than having to loop through the entire string before checking.

function isUpper(str) {
  let count = 0;

  for (let i = 0; i < str.length; i++) {
    if (/[a-zA-Z]/.test(str.charAt(i)) && str.charAt(i) == str.charAt(i).toUpperCase()) {
      count += 1;
    }

    if (count > 0) {
      return true;
    }
  }

  return false;

}

Let me know if this helps!

Upvotes: 1

Jhecht
Jhecht

Reputation: 4435

If you are checking to see if an entire string is uppercase, you can do something much easier.

function isUpper(str) {
  return /^[A-Z\s\W]+$/.test(str);
}

console.log(isUpper('ALL UPPER CASE'));
console.log(isUpper('ALL UPPER CASE! WITH ~@# STUFF'));
console.log(isUpper('THIS hAS ONE LOWER CASE CHARACTER'));
console.log(isUpper('THIS HAS 1 NUMBER'));

Explanation

The regex checks if the entire string from start (^) to end ($) matches the characters enclosed in the [], which is a range of from capital A through capital Z (A-Z, the special "space" character \s, and other non-word characters \W, matching one or more times. You could fine-tune this to make sure that it matches at least one capital letter, followed optionally by spaces, commas, exclamation points, etc. It all depends on your use cases and what the expected input vs. output would be

EDIT

Hadn't read the last sentence under the code. If all you want is to check for an uppercase character somewhere in the string, you could simply have

function isUpper(str) {
  return /[A-Z]/.test(str)
}

console.log(isUpper('ALL UPPER CASE'));
console.log(isUpper('ALL UPPER CASE! WITH ~@# STUFF'));
console.log(isUpper('THIS hAS ONE LOWER CASE CHARACTER'));
console.log(isUpper('THIS HAS 1 NUMBER'));
console.log(isUpper('this has no uppercase characters at all'));

Upvotes: 1

Related Questions