-
Notifications
You must be signed in to change notification settings - Fork 0
Java02. ДЗ 02, Васильев Роман #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
sproshev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8.5, в ответственностях сущностей я до конца так и не разобрался, доки может быть помогли бы
| FileOutputStream fos = new FileOutputStream(WORKDIR + "/" + TAGS); | ||
| ObjectOutputStream oos = new ObjectOutputStream(fos); | ||
| oos.writeObject(tags); | ||
| fos.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не всегда исполнение дойдет до сюда
-0.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oos нужно как минимум флашить в такой реализации
и лучше переменную fos не заводить, можно oos обойтись
|
|
||
| Set<String> getFiles() throws WorkspaceException; | ||
|
|
||
| List<String> get(String fileName) throws WorkspaceException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
плохое название, под списком строк может скрываться не только содержимое файла, но и что-то другое
| private final String directoryPath; | ||
|
|
||
| public FileWorkspace(String path) throws WorkspaceException { | ||
| this.directoryPath = path; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this не нужен
| return Files.walk(directoryPath).filter(path -> !path.equals(directoryPath)) | ||
| .map(path -> directoryPath.relativize(path).normalize().toString()).collect(Collectors.toSet()); | ||
| } catch (IOException e) { | ||
| throw new WorkspaceException(e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лучше все-таки не отбрасывать e, а передать его как причину в WException
|
|
||
| private static Tags restoreTags() throws IOException, ClassNotFoundException { | ||
| try (FileInputStream fis = new FileInputStream(WORKDIR + "/" + TAGS)) { | ||
| ObjectInputStream ois = new ObjectInputStream(fis); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
без fis можно обойтись
| private final Map<String, Node> nodes; | ||
|
|
||
| public InMemoryIndex(Map<String, Node> nodes) { | ||
| this.nodes = new HashMap<>(nodes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this
-0.5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А чем тут this плох? Здесь коллизия имен.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
да, тут согласен
| } else if (args.length == 2 && args[0].equals("branch")) { | ||
| tags.create(args[1], cvs.current()); | ||
| } else if (args.length == 2 && args[0].equals("remove")) { | ||
| tags.remove(args[1]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
что будет, если удалить активную ветку?
-0.5
| } else if (args.length == 2 && args[0].equals("merge")) { | ||
| String hash = tags.getHash(args[1]); | ||
| String newHash = cvs.mergeWith(hash); | ||
| tags.changeCurrent(newHash); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tags должны управляться системой контроля версий
| @SuppressWarnings("all") | ||
| public boolean saved() throws WorkspaceException { | ||
| return changedToCommit().isEmpty() && createdToCommit().isEmpty() && removedToCommit().isEmpty(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
используется только в тестах, там и должен жить
| throw new CVSException("" + hash + " contains untrack files."); | ||
| if (!changedToCommit().isEmpty() || !removedToCommit().isEmpty() || !createdToCommit().isEmpty()) | ||
| throw new CVSException("Not committed changes."); | ||
| if (changed().stream().filter(state::contains).findAny().isPresent()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anyMatch
@sproshev