Skip to content

HW02. GREP. Громов Павел#6

Open
PaGr0m wants to merge 2 commits into
devfrom
feature/grep-2
Open

HW02. GREP. Громов Павел#6
PaGr0m wants to merge 2 commits into
devfrom
feature/grep-2

Conversation

@PaGr0m

@PaGr0m PaGr0m commented Jun 7, 2020

Copy link
Copy Markdown
Owner

Предыдущий PR закрыл, так как в нем нет смысла (код был очень сильно переписан и была выбрана другая библиотека)

Выбор библиотеки:

  • В первый раз решено было выбрать библиотеку picocli, так как именно на нее наткнулся на хабре. Используя ее, понял, что ее функционал несколько оверхед. Опции приходилось указывать через аннотации (предполагаю, что внутри использовалась рефлекия, чтобы запоминать какие аргументы парсить).
  • В этот раз решено было воспользоваться библиотекой Commons CLI. Так как с ней уже встречались на курсе по Java и она оказалось удобной в данной реализации. Всего стоило добавить несколько опций и все. В отличии от picocli, в которой приходилось делать операций в разы больше.

В основном опирался на SO

@PaGr0m

PaGr0m commented Jun 7, 2020

Copy link
Copy Markdown
Owner Author

P.S. Вышел не самый лучший код... хотел сначала попробовать со стримами, как в прошлый раз, но столкнулся с проблемами, когда один флаг противоречит другому и как собирать строки при флаге A. Поэтому получился быдлокод :с

Изначально хотел сделать по примеру

private @NotNull Predicate<String> searchPredicate(@NotNull CommandLine command, String searchWord) {
    if (command.hasOption("i") && command.hasOption("w")) {
        return str -> str.toLowerCase().equals(searchWord.toLowerCase());
    } else if (command.hasOption("i")) {
        return str -> str.toLowerCase().contains(searchWord.toLowerCase());
    } else if (command.hasOption("w")) {
        return str -> str.equals(searchWord);
    }
    return str -> true;
}

public String run(@NotNull List<String> arguments) {
    ...
    List<String> result = rows.stream()
                              .filter(searchPredicate(cmd, searchWord))
                              .collect(Collectors.toList());
    ...
}

@ottergottaott ottergottaott left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почти отлично, но все еще падаем с исключением при неправильно вводе вместо вежливого сообщения

> grep -A -1 plugin build.gradle
> grep _a
> grep smth shell # когда директорию передали

*/
private String executeGrep(@NotNull CommandLine commandLine) {
List<String> argList = commandLine.getArgList();
String searchWord = argList.remove(0);

@ottergottaott ottergottaott Jun 8, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я вот только сейчас обратил внимание, что тут будут только вхождения искаться, но в задании требовалось:

поддерживающую регулярные выражения в строке поиска

Comment on lines +64 to +80
if (commandLine.hasOption("i") && commandLine.hasOption("w")) {
if (conditionCaseAndWordEquals(row, searchWord)) {
isAdd = true;
}
} else if (commandLine.hasOption("i")) {
if (conditionCase(row, searchWord)) {
isAdd = true;
}
} else if (commandLine.hasOption("w")) {
if (conditionWordEquals(row, searchWord)) {
isAdd = true;
}
} else {
if (conditionDefault(row, searchWord)) {
isAdd = true;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Попробуйте этот кусок вынести в отдельную функцию и заинлайнить вызовы condition<???>. Что-то вроде:

if (ignoreCase && onlyWords) {
    return Arrays.asList(row.toLowerCase().split(" ")).contains(searchWord.toLowerCase());
} else if (...) {
    ...
}

Мне это помогло заметить, что случаи, когда регистр не важен и нужно искать вхождение слов очень непохожи и независимы:

  1. в случае, когда игнорируем регистр, нужно все к одному регистру привести и ничего больше дополнительно не делать,
  2. в случае, когда нужно искать только слова, нужно сам поиск осуществлять по-другому.

А потому случаи должны быть разделены и код точно получится чище и проще.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. в случае, когда игнорируем регистр, нужно все к одному регистру привести и ничего больше дополнительно не делать

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

(хотя может я неправильно понял Ваш посыл)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вы можете оставить все, как сейчас есть, но вынести только выделенный кусок в функцию. Она будет принимать на вход текущую строку + какие-то еще параметры, и возвращать boolean:

  • true, если строка подходит под паттерн с флагами
  • false иначе

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

@PaGr0m

PaGr0m commented Jun 16, 2020

Copy link
Copy Markdown
Owner Author

Понял, что можно переделать реализацию на Pattern но почему-то появились проблемы с тем, чтобы вместе использовать два ключа

private int getPatternFlags(@NotNull CommandLine commandLine) {
    if (commandLine.hasOption("i") && commandLine.hasOption("w")) {
        return Pattern.CASE_INSENSITIVE | Pattern.CANON_EQ;
    } else if (commandLine.hasOption("i")) {
        return Pattern.CASE_INSENSITIVE;
    } else if (commandLine.hasOption("w")) {
        return Pattern.CANON_EQ;
    }

    return 0;
}

Поэтому сделал отдельно проверку на регистр и отдельно ConditionPredicate на ONLY_WORD. Как-то так)

Тесты:

SHELL >> grep
Command without arguments

SHELL >> grep 1
Wrong input

SHELL >> grep -A 2 plugin shell/build.gradle
plugins {
    id 'java'
}

SHELL >> 
Wrong input

SHELL >> grep apache shell/build.gradle
compile group: 'org.apache.logging.log4j', name: 'log4j-api', version: '2.13.0'
compile group: 'org.apache.logging.log4j', name: 'log4j-core', version: '2.13.0'

SHELL >> grep -w testCompile shell/build.gradle
testCompile group: 'junit', name: 'junit', version: '4.12'
testCompile "org.mockito:mockito-core:2.+"
testCompile "org.assertj:assertj-core:3.11.1"

SHELL >> grep testCompile shell/build.gradle
testCompileOnly 'org.projectlombok:lombok:1.18.12'
testCompile group: 'junit', name: 'junit', version: '4.12'
testCompile "org.mockito:mockito-core:2.+"
testCompile "org.assertj:assertj-core:3.11.1"

SHELL >> grep -i testannotationprocessor shell/build.gradle
testAnnotationProcessor 'org.projectlombok:lombok:1.18.12'

@ottergottaott

Copy link
Copy Markdown

Засчитываю

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants