HW01. CLI. Громов Павел by PaGr0m · Pull Request #1 · PaGr0m/ifmo-software-design

PaGr0m · 2020-03-03T23:27:30Z

ottergottaott

Напоминаю, что требовалось:

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

Из того, что не совсем корректно работает:

Не получается выполнить такой код

x=ex 
y=it
$x$y

cat не поддерживает считывание вывода предудещей команды в pipe:

echo aaaaaa | cat
SHELL >> File is not exist!

При попытке написать что-то с двойными кавычками, выпадаем с исключением:

x=1 
echo "123$x"
echo "1$x 23"
echo " ' $x' "

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

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

И постарайтесь, пожалуйста, создавать pr таким образом, чтобы в нем были только те изменения, которые относятся к текущей домашке

ottergottaott · 2020-03-10T21:16:21Z

    String name();

-    String run(String arguments, String options);
+    String run(List<String> arguments);


В целом, выделение команд в отдельную иерархию/использование паттерна команда (надеюсь, осознанное) здесь весьма разумно.

Но стоит подумать о том, действительно ли мы хотим из команд возвращать String, потому что это накладывает сильное ограничение: нельзя вернуть из команды частичный результат. Как следствие мы не можем одновременно выполнять команды в пайпе, всем последующим командам необходимо ждать, пока предыдущая полностью завершит работу.

Мб, сейчас это не самая большая проблема, но, конечно, на файлах >1гб проблема становится более очевидной.

Все-таки pipes немного иначе реализовал, а в команду передаю только аргументы

ottergottaott · 2020-03-10T21:36:21Z

+
+                commands.add(
+                        CommandEntity.builder()
+                                     .name(lexems.get(0).getWord())


Кажется, что в CommandEntity можно было класть не имя команды, а сразу саму команду. Так мы еще до начала выполнения, создания кучи объектов и т.д. можем узнать, что команды нет.

Исправил!

ottergottaott · 2020-03-10T21:42:56Z

+import java.util.List;

 public class CommandDefault implements Command {
    private String name = "";


Хорошо, что сделали поля с именами вместо литералов в командах.

Плохо, что в public String name() их не используете

Решил совсем избавиться от них. Единственное, только сделал конструктор у CommandDefault в который передаю название команды.

ottergottaott · 2020-03-10T21:46:05Z

 import java.util.Scanner;

 public class Shell {
    static Scanner scanner = new Scanner(System.in);


Не должно быт package-private полей

Ой, исправил!

ottergottaott · 2020-03-10T21:48:46Z


 public class Environment {
-
    private static String currentPath = System.getProperty("user.dir");


Советую быть крайне осторожным со всякого рода синглтонами. Их сложно поддерживать, они имеют тенденцию быстро разрастаться до god-object, их сложно тестировать.

И почти(вообще?) всегда от них можно избавиться

Избавился от синглтона, использую DI

ottergottaott · 2020-03-10T21:53:36Z

+            return output;
+        }
+
+        List<Lexem> lexems = parser.parseLexem(input);


Вообще в данном методе код в значительной степени повторяется. Уверен, что можно обобщить

Разнес логику

ottergottaott · 2020-03-10T21:54:02Z

 import java.util.List;

 public class Substitutor {
+    Environment environment = Environment.getInstance();


Опять package-private

Избавился от таких полей)

ottergottaott · 2020-03-10T21:55:21Z

+                continue;
            } else if (in.charAt(idx) == ' ') {
                if (word.length() > 0) {
                    lexems.add(Lexem.builder()


Здесь аналогично вижу частое дублирование + сам метод на два экрана

Исправлено

ottergottaott · 2020-03-10T21:56:21Z


    public Command getCommand(String commandName) {
-        return commands.getOrDefault(commandName, new CommandDefault());
+//        return commands.getOrDefault(commandName, new CommandDefault());


Не должно быть закомментированного кода

Вроде бы везде убрал комментирование

ottergottaott · 2020-03-10T21:57:38Z

-//
-//            System.out.println(output);
-            /////////////////////////////
+            System.out.print("SHELL >> ");


Мне, честно говоря, не нравится такой вывод. Обычно подобное SHELL >> используется как приглашение для ввода, но почему-то в Вашем случае печатается перед выводом :)

Да, тут просто косяк был. Думал, что с буфером что-то не так...

Исправил!

…mand.

Rework Shell CLI

PaGr0m · 2020-06-01T20:17:40Z

Обновил описание в начале

Что-то еще требуется по данному заданию?

ottergottaott

Все еще cat не поддерживает считывание вывода предудещей команды в pipe:

echo aaaaaa | cat
SHELL >> File is not exist!

Все еще не могу найти текстовое описание деталей реализации.

Код, в целом, стал лучше

ottergottaott · 2020-06-04T18:24:08Z

+        return CommandEntity.builder()
+                            .command(environment.getCommand(stringBuilder.toString()))
+                            .arguments(args)
+                            .build();


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

PaGr0m · 2020-06-04T23:59:15Z

Все еще cat не поддерживает считывание вывода предудещей команды в pipe:
echo aaaaaa | cat
SHELL >> File is not exist!
Все еще не могу найти текстовое описание деталей реализации.

Код, в целом, стал лучше

Да, забыл реализовать команду не для файла, исправил!

ottergottaott · 2020-06-05T14:58:41Z

Ок, засчитываю

PaGr0m added 4 commits March 3, 2020 17:46

Rework wc command. Rework shell. Add pipes command.

e1de01a

Complete commands. Add tests

4c46e14

Add tests.

9c74b3f

Gradle settings.

e32c43c

ottergottaott suggested changes Mar 10, 2020

View reviewed changes

PaGr0m added 5 commits May 31, 2020 23:37

Rework parser

9ff5e53

Rework parser and substitutor. Rework Command interface. Fix pipe com…

58e4a69

…mand.

Complete task 1 (shell)

2977d2a

Add class diagram

d926926

Merge pull request #5 from PaGr0m/feature/dev

c8b412c

Rework Shell CLI

ottergottaott suggested changes Jun 4, 2020

View reviewed changes

Rework cat command

03e2db5

Create readme file

16add6b


		public class Environment {

		private static String currentPath = System.getProperty("user.dir");

Conversation

PaGr0m commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ottergottaott left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PaGr0m Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PaGr0m commented Jun 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ottergottaott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PaGr0m commented Jun 4, 2020

Uh oh!

ottergottaott commented Jun 5, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PaGr0m commented Mar 3, 2020 •

edited

Loading

ottergottaott left a comment •

edited

Loading

PaGr0m Jun 1, 2020 •

edited

Loading

PaGr0m commented Jun 1, 2020 •

edited

Loading