это использование объектов избыточно и / или неэффективно?

Question

Я довольно неопытен в использовании объектов, поэтому мне очень хотелось бы получить какой-либо ввод.

Я пытаюсь удалить комментарии из списка, в которых есть определенные "нежелательные слова", как комментарии, так исписок «нежелательных слов» находится в объектах ArrayList.

Он находится внутри класса с именем FormHelper, который содержит закрытый член comments в качестве ArrayList, auditList ArrayList создается локально вфункция-член называется populateComments(), которая затем вызывает эту функцию (ниже).PopulateComments() вызывается конструктором, и поэтому эта функция вызывается только один раз, когда создается экземпляр FormHelper.

private void filterComments(ArrayList <String> auditList) {
    for(String badWord : auditList) {
        for (String thisComment : this.comments) {
            if(thisComment.contains(badWord)) {
                int index = this.comments.indexOf(thisComment);
                this.comments.remove(index);
            }
        }
    }
}

Что-то в том, как я реализовал, это не так,Я также обеспокоен тем, что я использую функции ArrayList неэффективно.Правильно ли мое подозрение?

Answer 1

Это не особенно эффективно. Однако найти более эффективное решение не так просто.

Давайте вернемся к более простой проблеме.

private void findBadWords(List <String> wordList, List <String> auditList) {
    for(String badWord : auditList) {
        for (String word : wordList) {
            if (word.equals(badWord)) {
                System.err.println("Found a bad word");
            }
        }
    }
}

Предположим, что wordList содержит N слов, а auditList содержит M слов. Некоторый простой анализ покажет, что внутренний цикл выполняется N x M раз. Коэффициент N неизбежен, но фактор M вызывает беспокойство. Это означает, что чем больше «плохих» слов вы должны проверить, тем дольше будет проверяться.

Есть лучший способ сделать это:

private void findBadWords(List <String> wordList, HashSet<String> auditWords) {
    for (String word : wordList) {
        if (auditWords.contains(word))) {
            System.err.println("Found a bad word");
        }
    }
}

Почему это лучше? Это лучше (быстрее), потому что HashSet::contains не нужно проверять все контрольные слова по одному. Фактически, в оптимальном случае он не проверит ни одного из них (!), А в среднем случае только один или два из них. (Я не буду вдаваться в причину, но если вы хотите понять, прочитайте страницу Википедии о хэш-таблицах.)

---

Но ваша проблема сложнее. Вы используете String::contains, чтобы проверить, содержит ли каждый комментарий каждое плохое слово. Это не простой тест на равенство строк (согласно моей упрощенной версии).

Что делать?

Ну, одно из возможных решений - разделить комментарии на массив слов (например, используя String::split, а затем использовать подход поиска HashSet. Однако:

• Это меняет поведение вашего кода. (На самом деле, в хорошем смысле: прочитайте проблему Scunthorpe !) Теперь вы будете сопоставлять только контрольные слова, если они являются реальными словами в тексте комментария.

• Разделение строки на слова не дешево. Если вы используете String::split, это влечет за собой создание и использование объекта Pattern для нахождения границ слов, создания подстрок для каждого слова и помещения их в массив. Вы, вероятно, можете сделать лучше, но это всегда будет нетривиальный расчет.

Таким образом, реальный вопрос будет в том, окупится ли оптимизация. В конечном итоге это будет зависеть от значения M; то есть количество плохих слов, которые вы ищете. Чем больше значение M, тем больше вероятность разбить комментарии на слова и использовать HashSet для проверки слов.

^{Другое возможное решение не предполагает разделения комментариев. Вы можете взять список контрольных слов и собрать их в одно регулярное выражение, например: \b(word-1|word-2|...|word-n)\b. Затем используйте это регулярное выражение с Matcher::find для поиска в каждой строке комментария плохих слов. Производительность будет зависеть от возможностей оптимизации движка regex на вашей платформе Java. Это может быть быстрее, чем расщепление.}

---

Мой совет - провести тестирование и профилирование всего приложения перед началом . Только оптимизировать:

1. когда бенчмаркинг говорит, что общая производительность запросов, где происходит проверка комментариев, касается. (Если все в порядке, не тратьте время на оптимизацию.)

2. , когда профилирование говорит, что этот метод является горячей точкой производительности. (Существует большая вероятность, что настоящие горячие точки находятся где-то еще. Если это так, вам следует оптимизировать их , а не этот метод.)

Обратите внимание, что существует предположение, что вы (достаточно) завершили свое приложение и создали для него реалистичный тест до того, как подумаете об оптимизации. (Преждевременная оптимизация - плохая идея ... если вы действительно не знаете, что делаете.)

Answer 2

Как правило, удаление отдельных элементов из ArrayList в цикле неэффективно, поскольку требует смещения всех «следующих» элементов на одну позицию в массиве.

A B C D E
  ^        if you remove this
    ^---^ you have to shift these 3 along by one
   / / /
A C D E

Если вы удалитемного элементов, это окажет существенное влияние на сложность времени.Лучше определить элементы, которые нужно удалить, а затем удалить их все сразу.

Я полагаю, что более точный способ сделать это - использовать removeIf, что (по крайней мере, для реализаций коллекций, таких как * 1007).*) выполняет это «все сразу» удаление:

this.comments.removeIf(
    c -> auditList.stream().anyMatch(c::contains));

Это сжато, но, вероятно, довольно медленно, потому что нужно постоянно проверять всю строку комментария, чтобы увидеть, содержит ли оно каждое плохое слово.

Вероятно, более быстрый способ будет использовать регулярное выражение:

Pattern p = Pattern.compile(
    auditList.stream()
        .map(Pattern::quote)
        .collect(joining("|")));
this.comments.removeIf(
    c -> p.matcher(c).find());

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

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

Answer 3

Вы пытаетесь изменить список, по которому вы перебираете.Ваш внутренний цикл (над комментариями) будет выбрасывать ConcurrentModificationException.Поэтому я предложу следующее изменение.

public void filterComment(List<String> auditList) {
   Iterator<String> it = this.comments.iterator();
   while ((it.hasNext())){
      String comment = it.next();
      for(String word : auditList){
        if(comment.contains(word)){
          it.remove();
          break;
        }
      }
   }
}