Помимо предположения о регулярном выражении (которое является хорошим), представляется более целесообразным иметь дело с массивами из символов , а не со строками с одним символом.
В частности, split("") вызов (дрожь) можно / нужно заменить на toCharArray(). Это позволяет вам перебирать каждого отдельного символа, что более четко указывает на ваши намерения, менее подвержено ошибкам, поскольку вы знаете , что вы обрабатываете каждый символ одновременно, и более эффективно *. Точно так же ваши действительные наборы символов также должны быть символами.
Ваша логика довольно странно выражена; вы даже не ссылаетесь на набор specialChars, и логика зацикливания, когда вы нашли совпадение, кажется странной. Я думаю, что это ваша ошибка; совпадение кажется неправильным в том смысле, что если символ соответствует действительному символу first , вы устанавливаете флаг на false и продолжаете вокруг текущего цикла; поэтому он определенно не будет соответствовать следующему действительному символу, и, следовательно, вы выйдете из цикла с флагом true. Всегда.
Я бы подумал, что-то вроде этого будет более интуитивно понятным:
private static final Set<Character> VALID_CHARS = ...;
public boolean isValidPhoneNumber(String number)
{
for (char c : number,toCharArray())
{
if (!VALID_CHARS.contains(c))
{
return false;
}
}
// All characters were valid
return true;
}
Это не учитывает последовательности (например, строки "-------- **" и "1" будут действительны, потому что все отдельные символы допустимы), но и ваш исходный код тоже не подходит. Регулярное выражение лучше, потому что оно позволяет вам указать шаблон, я приведу приведенный выше фрагмент в качестве примера более ясного способа перебора символов.
* Да, преждевременная оптимизация - корень всего зла, но когда лучше , чище код также оказывается быстрее, это дополнительный выигрыш бесплатно.