Количество вхождений подстроки в строку в Java

Сразу скажу, что код работает правильно, но наверняка не используется серьёзными программистами. Он предназначен для обучения школьников. Мне интересно мнение сообщества.

Вариант 1:

if (stringLength > 0 && subStringLength > 0 && stringLength >= subStringLength) {
    for (int i = 0; i < stringLength; i = index + subStringLength) {
        index = stringLowerCase.indexOf(subStringLowerCase, i);
        if (index < 0)
            break;
        count++;
    }
}

Вариант 2:

if (stringLength > 0 && subStringLength > 0 && stringLength >= subStringLength) {
    count = (stringLength - stringLowerCase.replace(subStringLowerCase, "").length()) / subStringLength;
}

Ответы (1 шт):

Автор решения: Stanislav Volodarskiy

Условия в if

if (stringLength > 0 && subStringLength > 0 && stringLength >= subStringLength) {
...
}

Условие stringLength > 0 не нужно. Если текст, в котором мы ищем, пуст, надо вернуть ноль. Он и без этого условия будет ноль.

Условие subStringLength > 0 выглядит разумно. Сколько раз пустая строка встретится в тексте? Этот вопрос кажется лишённым смысла, но в Java на него есть чёткий ответ: stringLength + 1. Пустая строка встречается между любыми двумя символами текста и ещё в его начале и конце. За этим ответом есть своя логика, не буду на неё отвлекаться. Вывод: условие имеет смысл, но надо обрабатывать и случай subStringLength == 0.

Условие stringLength >= subStringLength не нужно. Ситуация такая же как с условием stringLength > 0: код и без этого условия должен нормально работать.

Сложный for

for (int i = 0; i < stringLength; i = index + subStringLength) {

Это работающий код, но он нарушает нефункциональное требование "заголовок for самодостаточен, он не зависит от третьих переменных". Здесь index такая переменная. for – очень сложная конструкция, усложнять его ещё более внешними зависимостями не нужно. Вычисления в заголовке for могут зависеть от переменных, которые не меняются в цикле. stringLength и subStringLength – такие переменные. А вот индекс тут лишний.

Если нельзя записать for просто, его надо заменить на while.

Фигурные скобки

if (index < 0)
    break;

Всегда фигурные скобки в if, else, while, for. Отладка бывает тяжёлой, когда программист добавляет строку в тело такого if. Вам могут сказать, что в таких простых случаях скобки можно опустить, что они замусоривают код, но статистика говорит, что без скобок чаще болит голова.

subStringLength и subStringLowerCase, stringLength и stringLowerCase

Строки при приведении регистра меняют свою длину. Ваш код написан в предположении, что длина сохраняется. Это функциональная ошибка, баг сложный и неприятный.

Поиск без учёта регистра сломан

Это функциональная ошибка. Почитайте описание compareToIgnoreCase. Там написано, что надо приводить строку к верхнему регистру, затем к нижнему. И это ещё не вся история. На самом деле UNICODE определяет понятие casefold, специально предназначеное для сравнения строк без учёта регистра, но без специальных библиотек в Java этого не сделать. Так что хотя бы надо приводить вверх-вниз.

Подсчёт через цикл

public class Temp {
    public static String toCasefold(String str) {
        return str.toUpperCase().toLowerCase();
    }

    public static int countSubstringsIgnoreCase(String text, String str) {
        text = toCasefold(text);
        str = toCasefold(str);
        int textLength = text.length();
        int step = Math.max(1, str.length());

        int count = 0;
        int pos = 0;
        while (pos <= textLength) {
            int index = text.indexOf(str, pos);
            if (index < 0) {
                break;
            }
            pos = index + step;
            ++count;
        }
        return count;
    }

    public static void show(String text, String str) {
        int count = countSubstringsIgnoreCase(text, str);
        System.out.println("\"" + text + "\", \"" + str + "\", " + count);
    }
    
    public static void main(String[] args) {
        show("", "");
        show("a", "");
        show("abc", "");
        show("abc", "abc");
        show("abcabc", "abc");
        show("abc", "abcd");
        show("abababababababababa", "aba");
    }
}

Подсчёт через replace

Достаточно заменить только один метод:

    public static int countSubstringsIgnoreCase(String text, String str) {
        text = toCasefold(text);
        str = toCasefold(str);

        int textLength = text.length();
        int strLength = str.length();
        
        if (strLength == 0) {
            return textLength + 1;
        }
        return (textLength - text.replace(str, "").length()) / strLength;
    }

Анализ

Правильность и скорость

Про правильность поиска без учёта регистра я уже писал выше.

Оба метода работают правильно (если не думать про турецкий алфавит) и относительно быстро (если не брать длинные строки). Оба метода можно заставить работать за квадратичное время на длинных строках. Есть более продвинутые методы поиска подстрок, но их надо искать в библиотеках.

Вы выбрали задачу которая с одной стороны практически полезна. А с другой стороны, чтобы решить её полностью правильно и быстро вам понадобятся две библиотеки: одна для UNICODE, вторая для быстрого поиска подстроки.

Стиль кода

Отыщите документ по стилю написания кода, например Google Java Style Guide, и следуйте ему в оформлении кода (я, кстати, так не делаю, потому что я плохой ученик). И ученикам скажите что мы на уроках следуем вот этому документу. Это будет важно для командной работы. Сейчас у вас не так.

Хороший код - короткие строки. Каждая строка делает что-то одно. Поэтому я написал замечание про for.

Примеры должны быть полными. В вопросе у вас обрывки кода, в итоге вы решаете не всю задачу, а только её основную часть. А нужно чтобы все краевые случаи были обработаны (как например пустые строки) или задокументированы.

Снабжайте код примерами использования для всех основных и краевых случаев. Объём кода и сложность решения меняются кардинально в зависимости от того насколько оно полное.

P.S. Это не задача, а ящик Пандоры какой-то.

→ Ссылка