Skip to content

Conversation

@nizshee
Copy link
Owner

@nizshee nizshee commented Sep 25, 2016

Copy link

@sproshev sproshev left a 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();

Choose a reason for hiding this comment

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

не всегда исполнение дойдет до сюда

-0.5

Copy link

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;

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;

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());

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);
Copy link

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);
Copy link

Choose a reason for hiding this comment

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

this

-0.5

Copy link
Owner Author

Choose a reason for hiding this comment

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

А чем тут this плох? Здесь коллизия имен.

Copy link

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]);
Copy link

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);
Copy link

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();
}
Copy link

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())
Copy link

Choose a reason for hiding this comment

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

anyMatch

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