Checking a string for invalid characters

Question

I am trying to write a program that takes a string as input.

This string must be an equation with only brackets, operations or digit.

The code I posted below is what I came up with, but when I run it, anything I enter is apparently an invalid string. Can someone please tell what I did wrong?

        System.out.println("Enter a string");                               
        String str = s.nextLine();   //s is a Scanner object                    
        int n = str.length();                                           

        for(int i = 0; i < n; i++) {                                        
            if( !Character.isDigit(str.charAt(i)) || str.charAt(i) != '+'
                || str.charAt(i) != '-' || str.charAt(i) != '*'
                || str.charAt(i) != '/' || str.charAt(i) != '('
                || str.charAt(i) != ')' || str.charAt(i) != '['
                || str.charAt(i) != ']' || str.charAt(i) != '{'
                || str.charAt(i) != '}') {
                    System.out.println("invalid string, try again: ");
                    str = s.nextLine();     
            }
        ...}

Every character is not '-' OR not '+'. I think you wanted && not || . — azurefrog, Jan 24 '18 at 21:38
You're asking Java to tell you if it isn't a digit or isn't a + or isn't a - etc... and if any of those are true it's an error. Are you sure you want `||`? — Dave Newton, Jan 24 '18 at 21:38
Your `||`s should be `&&`s, and also when you find an invalid character you just get a new string from input and then continue the loop - you need to break out of the loop and start the whole thing again. — Blorgbeard, Jan 24 '18 at 21:38
Unrelated, but put the corrected logic into a separate function like `isValidChar` or something to help the mainline code be legible. — Dave Newton, Jan 24 '18 at 21:39
In one step you can check if your input is valid or not, you can use regular expression with matches `boolean check = str.matches("[\\d+*/\\(\\)\\[\\]\\{\\}-]+");` — Youcef LAIDANI, Jan 24 '18 at 21:41
Also unrelated, but it annoys me when people hand out answers to questions like this instead of simply encouraging some thought and exploration :( — Dave Newton, Jan 24 '18 at 21:43
Regular expression is bettered suited for this, to get started see [here](https://stackoverflow.com/questions/11009320/validate-mathematical-expressions-using-regular-expression) — SomeDude, Jan 24 '18 at 21:44
Also, it's typical to allow white space ("1 + 2" instead of just "1+2") in an equation, and your specification doesn't allow that. Plus, an equation with unbalanced parenthesis ("1)+2") will still pass your test. I think it might be better to just go directly to parsing and not bother with "characters." — markspace, Jan 24 '18 at 21:47
It's incredible how often this issue comes up and how hard it is to find a suitable dupe question :/. — Tom, Jan 24 '18 at 21:47
@svasa I'd disagree. It's a horrible regex (particularly in Java) when a contains or actual parsing will do. Maintainability and legibility almost always trumps anything else. — Dave Newton, Jan 24 '18 at 21:47
What's the usual meme? "I have this problem". "Here, use this regex". "Now I have these two problems". — Jason, Jan 24 '18 at 21:48
@Jason `Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems.` https://blog.codinghorror.com/regular-expressions-now-you-have-two-problems/ — markspace, Jan 24 '18 at 21:49
(Noting that I use regex a lot; they're an essential tool--so is a hammer, but I don't use it to drive screws.) — Dave Newton, Jan 24 '18 at 21:49
@YCF_L Braces and parenthesis are not special inside a character class: `[\\d+*/()\\[\\]{}-]+` — Andreas, Jan 24 '18 at 21:50

score 4 · Answer 1 · answered Jan 24 '18 at 21:42

As suggested in the comments, switch || for &&:

if( !Character.isDigit(str.charAt(i)) && str.charAt(i) != '+'
        && str.charAt(i) != '-' && str.charAt(i) != '*'
        && str.charAt(i) != '/' && str.charAt(i) != '('
        && str.charAt(i) != ')' && str.charAt(i) != '['
        && str.charAt(i) != ']' && str.charAt(i) != '{'
        && str.charAt(i) != '}') {
    System.out.println("invalid string, try again: ");
    str = s.nextLine();     
}

Or, to make your code easier to understand:

final String validChars = "0123456789+-*/()[]{}";
for(int i = 0; i < n; i++) {
    if(!validChars.contains(str.charAt(i))) {
        System.out.println("invalid string, try again: ");
        str = s.nextLine();
        // potential bug here - i and n are not being reset
    }
}

Note: You have a bug where you don't reset the i index or the n length when reading a new line from the scanner in the case where your previous line contained an invalid character (see comment in code above).

@Franklin If this answer resolved your problem, then please consider accepting it, by clicking the green checkmark to the left. — Jason, Jun 18 '19 at 06:01

Albanninou · Answer 2 · 2018-01-24T21:54:49.357

if you dont want change all of your code you can that :

 System.out.println("Enter a string");

    String str = s.nextLine(); // s is a Scanner object
    int n = str.length();

    if (n > 0) {
        for (int i = 0; i < n; i++) {
            if (!(Character.isDigit(str.charAt(i)) || str.charAt(i) == '+' || str.charAt(i) == '-'
                    || str.charAt(i) == '*' || str.charAt(i) == '/' || str.charAt(i) == '(' || str.charAt(i) == ')'
                    || str.charAt(i) == '[' || str.charAt(i) == ']' || str.charAt(i) == '{'
                    || str.charAt(i) == '}')) {
                System.out.println("invalid string, try again: ");
                str = s.nextLine();
                n = str.length();
                i = -1;//reset i for the new line ( set at -1 for the next i++ of the next loop )
            }
        }
    } else {
        System.out.println("empty string ");
    }

That changes all of the code, just in a different way :/ – Dave Newton Jan 24 '18 at 21:48 — Dave Newton, Jan 24 '18 at 21:48

score 0 · Answer 3 · answered Jan 24 '18 at 22:09

package hello;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

import org.junit.Test;

public class Characts {

    @Test
    public void testSwitch () {
         assertTrue(isValid("123+321*"));
         assertFalse(isValid("A123+321*"));
         assertTrue(isValid("123+321*\n")); // don't forget returns and line feeds
    }

    private boolean isValid(String str) {
        for (int i = 0 ; i < str.length(); i++) {
            char s = str.charAt(i);
            if (Character.isDigit(s)) {
                continue;
            } else {
                switch (s) {
                case '+' :
                case '-' :
                case '*' :
                case '/' :
                case '(' :
                case ')' :
                case '[' :
                case ']' :
                case '{' :
                case '}' :
                    continue;
                }
                return false;
            }
        }
        return true;
    }
}

Checking a string for invalid characters

3 Answers3