task1 #3
task1 #3
Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
- файлы .class и другие продукты сборки не надо выкладывать в систему контроля версий
- нужна ещё диаграмма и текст про архитектуру
С функциональной точки зрения всё зашибись, но с кодом какая-то беда. Поправьте, пожалуйста :)
|
|
||
| /** | ||
| * Добавляет переменную в окружение | ||
| * @param key - имя переменной |
There was a problem hiding this comment.
Дефисы лучше не писать, JavaDoc сам их генерит (а так сгенерит два, думая, что дефис --- часть комментария)
| * @return - значение переменной | ||
| */ | ||
| public static String get(String key) { | ||
| if(table.containsKey(key)) { |
There was a problem hiding this comment.
По стайлгайду положен пробел перед (
| @@ -0,0 +1,37 @@ | |||
| package environment; | |||
There was a problem hiding this comment.
Имя пакета неканоничноъ. https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html
| } | ||
|
|
||
| public Cat() { | ||
| this.args = null; |
There was a problem hiding this comment.
По идее, он и так null, Java-машина гарантирует инициализацию всех полей нулями
| /** | ||
| * Класс-обёртка команды cat | ||
| */ | ||
| public class Cat extends CommandInterface{ |
There was a problem hiding this comment.
По стайлгайду положен пробел перед {
| @@ -0,0 +1,66 @@ | |||
| package prepair; | |||
There was a problem hiding this comment.
prepair --- это нечто среднее между prepare и repair? :)
| /** | ||
| * Класс для интерполяции строк | ||
| */ | ||
| public class Interpol { |
| outContent.reset(); | ||
| t = m.getPrintThread(); | ||
| t.join(mills); | ||
| assertEquals("1\n2 3\n4 5 6\n", outContent.toString()); |
There was a problem hiding this comment.
Лучше разделить эти тест кейсы на несколько тестов. Принцип независимости тестов же.
|
|
||
| @DisplayName("test cat") | ||
| @Test | ||
| void test2() throws BadQuotesException, IOException, InterruptedException { |
There was a problem hiding this comment.
| System.setErr(System.err); | ||
| } | ||
|
|
||
| public void runManeger(String input) throws BadQuotesException, IOException { |
There was a problem hiding this comment.
Даже в именах очень много опечаток. Большая часть кода, наверное, была написана поздно ночью :)
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Ещё нужно было краткое обоснование выброа библиотеки для разбора параметров, в виде текста где-нибудь в корневой папке проекта
|
|
||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| class GrepTest extends TestCommand{ |
| "from (\n" + | ||
| " select GENERATE_SERIES \n" + | ||
| " from GENERATE_SERIES('2012-08-01'::date,'2012-08-31','1 day')) d\n", outContent.toString()); | ||
| } |
There was a problem hiding this comment.
Каждый тестовый сценарий идеологически должен быть в отдельном тесте. Чтобы если один тест не прошёл, то сразы было бы видно, что именно не так.
| /** | ||
| * Препроцессинг, инициализурующий функцианальные поля grep для различных ключей | ||
| * @throws ParseException | ||
| * @throws NumberFormatException |
There was a problem hiding this comment.
Тут тоже неплохо бы комментарии
| errOutput.write(errBuilder.toString().getBytes()); | ||
| errOutput.flush(); | ||
| output.close(); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
И тут тоже проглатывать исключения нехорошо
|
|
||
| } | ||
|
|
||
| private void evalGrep (String[] args, PipedOutputStream output, PipedInputStream input, PipedOutputStream errOutput) { |
There was a problem hiding this comment.
Пробел перед "(" после имени функции не ставится
| } else if (counter <= lineCount) { | ||
| output.write((prefix + line + "\n").getBytes()); | ||
| } | ||
| counter ++; |
There was a problem hiding this comment.
А вот унарные операторы обычно не отделяются, правильнее counter++;
| private void printHelp (PipedOutputStream errOutput) { | ||
| StringWriter out = new StringWriter(); | ||
| PrintWriter writer = new PrintWriter(out); | ||
| helpFormatter.printHelp(writer,80,"grep",null,posixOptions,1,4,null,true); |
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Good enough. CLI и grep зачтены
| @@ -0,0 +1 @@ | |||
| Ответ очень прост: когда-то давно я уже пользовался этой библиотекой и поэтому знаю что к чему в ней. А новую изучать не вижу смысла, если и эта хорошо себя показывает. No newline at end of file | |||
There was a problem hiding this comment.
"Золотой Молоток". Примерно как "почему я программирую на Java? Потому что я всю жизнь программировал на Java, и отец мой программировал на Java, и дед мой программировал на Java, и никогда с ней проблем не было" :)
| outContent.reset(); | ||
| t = m.getPrintThread(); | ||
| t.join(mills); | ||
| assertEquals("1\n2 3\n4 5 6\n", outContent.toString()); |
No description provided.