Skip to content

Added ls and cd commands - HW3#2

Open
inspired99 wants to merge 2 commits intoZinovyevaAnna:devfrom
inspired99:HW3
Open

Added ls and cd commands - HW3#2
inspired99 wants to merge 2 commits intoZinovyevaAnna:devfrom
inspired99:HW3

Conversation

@inspired99
Copy link

Архитектурные решения мне показались очень даже логичными и понятными, по коду достаточно легко ориентироваться, расширить функциональность оказалось не затруднительно. Понравилась фича с возвратом статуса для каждой команды, что довольно удобно. Для добавления новой команды нужно всего лишь реализовать 2 метода (в конструкторе команд и у самой команды) и добавить case в switch.

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

Copy link

@FirstTimeInForever FirstTimeInForever left a comment

Choose a reason for hiding this comment

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

На удивление работает правильно. 10/10.

File isRealPathAbs = new File(newPath);
File isRealPath;
if (Objects.requireNonNull(environment.get("PWD")).endsWith("/")) {
isRealPath = new File(environment.get("PWD") + newPath);

Choose a reason for hiding this comment

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

Лучше использовать более современный nio.Path. Тогда достаточно удобно можно вызвать метод resolve() для достижения почти любых целей)

}
}

if (pathToCheck == null) {

Choose a reason for hiding this comment

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

Наверное стоило бы разбить этот метод на несколько более мелких и простых.

@FirstTimeInForever
Copy link

@ZinovyevaAnna Вам нужно добавить исключение для PWD и не делать фолбэк на внешние проперти (так вы наследуете cwd от шелла, который запустил процесс который запускает ваш шелл). Достаточно просто единожды в самом начале проинициализировать его значением System.getProperty("user.dir").

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