Как можно улучшить код: «Ввести текст из файла набор предложений»?

#java #инспекция_кода

Дали тестовое задание:

Ввести текст из файла набор предложений. Каждое предложение в
отдельной строке, слова разделены пробелом. Вывести количество слов во
всех предложениях, начиная со второго, в которых есть хотя бы одно
слово из первого предложения.

Так же выдает дубликаты в результатах:

(11)Жизнь — это больница, где каждый пациент мечтает перебраться на другую кровать.

(13)Жизнь не в том, чтобы жить, а в том, чтобы чувствовать, что живешь.

(13)Жизнь не в том, чтобы жить, а в том, чтобы чувствовать, что живешь.


Ниже моя реализация. Пожалуйста, подскажите, можно ли улучшить мой код? Если да,
как это сделать? Пожалуйста, укажите почему то, что вы рекомендуете является улучшением?

TextFromFile 

package main.java;

public class TextFromFile {
  public static void main(String[] args) {
    new ReadFromFile().read();
  }
}


ReadFromFile

package main.java;

import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

public class ReadFromFile {

  private final String FILE_NAME = "src/main/resources/text.txt";
  private List stringsText;
  private String firstSentence;
  private String[] firstSentenceArray;

  public void read() {

    stringsText = new ArrayList<>();
    Scanner in = null;
    try {
      in = new Scanner(new File(FILE_NAME));
    } catch (FileNotFoundException e) {
      e.printStackTrace();
    }

    if (in.hasNext()){
      firstSentence = in.nextLine() + "\r\n";
      firstSentenceArray = firstSentence
          .replaceAll("[^а-яА-Я a-zA-Z]", " ")
          .replaceAll("\\s+", " ")
          .split(" ");
    }

    while(in.hasNext()) {
      String str = in.nextLine() + "\r\n";
      stringsText.add(str);
    }
    in.close();

    WordSearch wordSearch = new WordSearch();
    for (String aStringsText : firstSentenceArray) {
      stringsText.forEach(text -> wordSearch.wordsearch(text, aStringsText));
    }
  }
}


WordSearch.java

package main.java;

public class WordSearch {

  public void wordsearch(String text, String searchText) {
    String tt = text.replace("—", "")
        .replace("-", "")
        .replace(",", "");
    String[] textArray = tt.split(" ");

    for (String word : textArray) {
      if (searchText.equalsIgnoreCase(word)){
        String result = tt.replaceAll("\\s+", " ");
        int cnt = result.split(" ").length;
        System.out.print("(" + cnt + ")");
        System.out.println(text);
      }
    }

  }
}

    

Ответы

Ответ 1

} catch (FileNotFoundException e) {
  e.printStackTrace();
}



После этого выполнение продолжится и всё свалится на if из-за null.


firstSentenceArray = firstSentence
    .replaceAll("[^а-яА-Я a-zA-Z]", " ")
    .replaceAll("\\s+", " ")
    .split(" ");



Какая-то муть. Что мешает делать одну замену по регулярке, а не две? Что мешает сразу
делать split по регулярке? Что если строка начинается или заканчивается пробелом?


private final String FILE_NAME = "src/main/resources/text.txt";



Кажется, эту строку надо передавать в конструктор.



Ну и всё, что в ответе @vp_arth.


Ответ 2

Токенизировать слова лучше так: .split("\\P{L}+").  

То, что вы по разному интерпретируете понятие "слово" в первом и остальных предложениях
- это косяк. Тут должна быть консистентность - вынесите токенизацию в отдельный метод.

Алгоритмическая сложность квадратична(линейный wordSearch  в цикле по firstSentenceArray).
Проверку наличия слова в первом предложении можно сделать за константное время, если
применить подходящую структуру данных(HashSet, например).

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



На мой взгляд реализация должна выглядеть как-то так(псевдокод):  

sentence = readSentence();
map = tokenize(sentence);
while (hasInput()) {
  sentence = readSentence();
  words = tokenize(sentence);
  for (word in words) {
    if (map.contains(word)) {
      print words.length, sentence;
      break;
    }
  }
}


Ответ 3

Во-первых, хочу отметить, что у вас распух readfile. Это же по факту read-and-parse-and-something-and-and....
Для начала дайте корректное имя, отражающее смысл действий, а потом задумайтесь о декомпозиции
на отдельные примитивы, чтобы не было слишком много ответственностей в одном месте.

Во-вторых, не надо хардкодить параметры! Почему имя файла не параметр, а константа?

В-третьих, давайте переменным осмысленные имена. Окей, я сразу понял, что такое firstSentence,
это хороший образец. А что такое 'in'? Я думал, думал... И нашел! Это (неожиданно)
сканер! Давайте так: на заборе должно быть написано "дрова" и лежать там должны тоже дрова