Conversation
FirstTimeInForever
left a comment
FirstTimeInForever
left a comment
There was a problem hiding this comment.
- Не работает подстановка:
>> x=ex
>> y=it
>> $x$y
Cannot run program "ex$y": error=2, No such file or directory
-1.
wcнекорректно считает:
# coreutils
wc README.md
18 85 1129 README.md
# yours
>> wc README.md
19 89 1111 README.md
-0.5.
- директория которую выводит
pwdне совпадает с настоящей директорией:
# I've ran your shell from the IDEA gutter icon
>> pwd
/home/ivm/.local/share/JetBrains/Toolbox/apps/IDEA-U/ch-1/223.172/bin
# External ls shows files in project root (not the directory above^^^)
>> ls
build
build.gradle.kts
gradle
gradlew
gradlew.bat
README.md
settings.gradle.kts
src
-1.
- Пустая переменная раскрывается в
null:
>> echo "$a"
null
-0.25.
- Нет
.gitignoreи адекватного.editorconfig.-0.5. - Нет линтера (для джавы советую
checkstyle).-0.5.
Это первая работа в которой работают continuations для кавычек. Поздравляю)
Большая часть окей, пожалуйста исправьте замечания (пишите сразу в тг когда проверять).
5.75/10.
build.gradle.kts
Outdated
| java | ||
| } | ||
|
|
||
| group = "org.example" |
There was a problem hiding this comment.
Тут и в версии можно было бы что-нибудь написать)
| @@ -1 +1,19 @@ | |||
| # sd-homework No newline at end of file | |||
| # simplecli | |||
There was a problem hiding this comment.
Не хватает только инструкции по запуску. -0.5.
|
|
||
| import java.util.List; | ||
|
|
||
| public interface CommandConstructor { |
There was a problem hiding this comment.
А зачем эта абстракция? Казалось бы это может вам помочь абстрагировать создание команд, но на самом деле именно эту задачу решает фабрика, которая у вас уже есть.
| } | ||
| var name = args.get(0); | ||
| return switch (name) { | ||
| case "cat" -> new Cat().construct(args.subList(1, args.size()), environment); |
There was a problem hiding this comment.
Достаточно сделать Cat не CommandConstructor, а Command и конструировать его напрямую тут (return new Cat(...)). Сейчас у вас по-факту получилась фабрика, которая использует еще по фабрике на каждую команду (у вас это не абстрактные фабрики, поэтому получается просто много лишнего кода).
| /** | ||
| * Class to construct commands | ||
| */ | ||
| public class CommandFactory { |
There was a problem hiding this comment.
В идеале нужно сделать абстракной фабрикой и параметризировать ей то, что использует этот код.
| * Class for anything that can be executed | ||
| */ | ||
| public interface Executable { | ||
| enum EndStatus { |
There was a problem hiding this comment.
На самом деле было бы логично использовать exit code тут, но это тоже окей в этом проектe.
| protected String output = null; | ||
| protected EnvironmentManager environment; | ||
|
|
||
| public Command(List<String> args, EnvironmentManager environment) { |
There was a problem hiding this comment.
Гораздо лучше было бы передавать сюда еще и IO потоки. Тогда очень сильно облегчается тестирование и можно заимплементить более оптимальный конвеер для команд с пайпами.
| assertEquals(Executable.EndStatus.SUCCESS, cmd.getEndStatus()); | ||
| assertEquals("test file", cmd.getOutput().trim()); | ||
|
|
||
| cmd = new Cat().construct(List.of(path + "testfile2"), env); |
There was a problem hiding this comment.
Обычно разные сценарии разбивают на разные методы. В случаее если ассерты выше провалятся, то эти проверки не будут выполнены. Еще можно использовать SoftAssertions, но в этом случае они не очень уместны.
|
|
||
| cmd = new Cat().construct(List.of(path + "testfile2"), env); | ||
| cmd.execute(null); | ||
| assertEquals(Executable.EndStatus.SUCCESS, cmd.getEndStatus()); |
There was a problem hiding this comment.
Общую тестовую логику можно вынести в отдельный метод. Можно даже сделать базовый класс для тестов и от него наследовать конкретные тесты!
2 participants
No description provided.