LiteVCS#9
Conversation
| @@ -0,0 +1,19 @@ | |||
| Список команд: | |||
There was a problem hiding this comment.
такая штука тоже полезна, но это пользовательская информация. под кратким описанием внутреннего устройства в условии имелось в виду описание архитектуры
There was a problem hiding this comment.
Добавил описание архитектуры
| for (String path : e.getConflicts()) { | ||
| System.out.println(" " + path); | ||
| } | ||
| } catch (LostFileException e) { |
There was a problem hiding this comment.
а почему бы все эти сообщения не заворачивать внутрь объекта исключения при его создании? тогда тут можно будет сделать catch по базовому классу и просто вывести сообщение
There was a problem hiding this comment.
Ну, идея была в том, чтобы максимально вынести логику написания сообщений из библиотеки. Вдруг пользовательский интерфейс на другом языке будет, например?
There was a problem hiding this comment.
локализация другими средствами делается -- обычно текстовые строки выносятся в отдельные файлы, и подменяются для нужного языка. на уровне кода с этим оооочень геморройно будет работать. тут важнее определить, кто конкретно знает, что за проблема произошла, и это скорее всего именно то место, которое создаёт объект исключения
| private LiteVCS() { | ||
| } | ||
|
|
||
| public static void hello(@NotNull String path, @NotNull String author) throws BrokenFileException, |
There was a problem hiding this comment.
этому методу явно не хватает документации
| */ | ||
| public static void init(@NotNull String path) throws RecreatingRepositoryException, | ||
| RepositoryNotInitializedException { | ||
| new DataManager(path).initRepository(); |
There was a problem hiding this comment.
очень интересно будет посмотреть, как вы будете модульные тесты к такому коду писать :)
| ContentDescriptor stage = dataManager.getStage(); | ||
| ContentDescriptor updatedStage; | ||
| if (!file.isFile()) { | ||
| throw new Error("Possible to add files only"); |
There was a problem hiding this comment.
An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch (source)
Error это обычно разного рода аппаратные ошибки, ошибки при работе с памятью и т.п. в своём коде лучше использовать RuntimeException
| */ | ||
| public static void createBranch(@NotNull String path, @NotNull String branchName) | ||
| throws BrokenFileException, LostFileException, ConflictNameException, RepositoryNotInitializedException { | ||
| DataManager dataManager = new DataManager(path); |
There was a problem hiding this comment.
вот смотрите, паттерн в том, что каждый метод тут начинается с создания DataManager'а. а можно было бы хранить в себе просто локально экземпляр и вызывать у него нужные методы, передавая им правильный путь. и тогда эта зависимость была бы явно видна, и её можно было бы пытаться замокать в тестах
|
|
||
| private static final String ROOT_DIRECTORY_NAME = ".liteVCS"; | ||
| private static final String PATH_TO_VERSIONS_FILES = Paths.get(ROOT_DIRECTORY_NAME, "versions").toString(); | ||
| private static final String PATH_TO_CONTENT_DESCRIPTORS_FILES = Paths.get(ROOT_DIRECTORY_NAME, "descriptors").toString(); |
| * Special class which provides all the main | ||
| * functionality of library by static methods | ||
| */ | ||
| public class LiteVCS { |
There was a problem hiding this comment.
вообще мне нравится, как у вас получилось разделить логику от работы с файлами. теперь нужно сделать этот класс более тестируемым
There was a problem hiding this comment.
Вынес DataManager в поле, сделал все основные методы не статическими. В конструктор можно передавать DataManager. Можно для тестов подсунуть свой, фейковый.
|
|
||
| import static org.junit.Assert.*; | ||
|
|
||
| public class DataManagerTest { |
|
|
||
|
|
||
| @Test | ||
| public void addBranchSwitchChangeSwitchMerge() throws Exception { |
There was a problem hiding this comment.
два больших сценария -- это хорошо, но хотелось бы ещё кучку сценариев поменьше + модульные тесты всё же
There was a problem hiding this comment.
Добавил маленьких тестов
|
✗ java -jar ../liteVCS-1.0-SNAPSHOT.jar init я что-то сделал не так? |
|
А там есть такой файл? Я что-то типа *.fgvvv не делал. Надо сделать? |
|
есть конечно. я переименовал файл, чтобы в его имени не было точки, ошибка та же выдалась. оно от меня чего-то особого хочет? |
Added information about lite VCS
|
✗ java -jar ../my_svn-1.0-SNAPSHOT.jar init a пишет еррор, а репозиторий всё равно инициализирует UPD: а, нет. оно только папку с логами создаёт. и это мешает потом проинициализировать репозиторий уже по-нормальному: ✗ java -jar ../my_svn-1.0-SNAPSHOT.jar init |
|
лог писать прямо в рабочую копию -- это не айс, конечно. особенно если его ещё и в выводе команды status показывать постоянно пользователям. давайте их хотя бы из статуса уберём? |
|
очень хочется какой-то хелп со списком команд и аргументов |
|
ещё пусть status выводит текущий бранч, а то хрен разберёшься, где находишься |
|
сделал два бранча с разными файлами, замерджил один в другой, файла второго не увидел. хотя может я что не так делал, хелпа по командам то нет |
|
Логи по плану пишутся в .liteVCS/logs и, конечно, не отображаются в status. Была проблема с различием форматов путей к файлам в Windows и Linux, поэтому раньше её не замечал. Протестировал в виртуальной машине. Теперь должно работать как надо) |
|
Название активной ветки в status выводит |
|
help [название команды] выводит описание команды |
|
help --all выводит список всех команд |
|
При слиянии веток, пользовательские файлы не обновляются. Чтобы увидеть изменения надо сделать reset [file] или checkout [id последнего коммита в ветке] |
|
в итоге по 9 баллов за обе задачи из-за большого количества замечаний и ошибок по ходу |
2 participants
No description provided.