Skip to content

Hw3 trie serializtion#3

Merged
danlapko merged 4 commits intomasterfrom
hw3_trie_serializtion
Apr 27, 2018
Merged

Hw3 trie serializtion#3
danlapko merged 4 commits intomasterfrom
hw3_trie_serializtion

Conversation

@danlapko
Copy link
Copy Markdown
Owner

Copy link
Copy Markdown

@dsavvinov dsavvinov left a comment

Choose a reason for hiding this comment

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

Неплохая работа. Валидации инпута нет, но из-за того, как устроено внутри дерево, не совсем и понятно, как валидировать (кроме как с помощью checksum)
10б, зачтено

@@ -0,0 +1,140 @@
package hw1;

import com.sun.istack.internal.Nullable;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не те аннотации. Можно брать, например, из пакета org.jetbrains.annotations

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А, это вообще не тот код. Не надо так, пожалуйста.

@@ -0,0 +1,31 @@
package hw2;

public interface Dictionary {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему в этом pull request эти файлы?

final Node[] children = new Node[ALPHABET_SIZE];
boolean isTerminal = false;
int numberOfSubTerminals = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code-style: пустые строчки

public class TrieImpl implements Trie, StreamSerializable {
private Node root;

private static int charToInt(char ch) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor: я бы предложил такой метод положить в конец класса, т.к. это явно не то, что читатель кода хочет увидеть в первую очередь.

}


static class Serializer {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍
Пока что единственный человек, который вынес сериализацию/десериализацию в отдельный класс (что я поддерживаю, т.к. функционал сериализации/десериализации как бы немного "сбоку" от основного функционала ноды)



if (type == -1) {
throw new hw3.SerializationException();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Здесь хорошо бы добавить сообщение, объясняющее, что стряслось и как с этим жить.


static void serialize(Node node, OutputStream out) throws IOException {
if (node == null) {
out.write(new byte[]{NULL_NODE});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FYI, у OutputStream есть перегрузка write, которая собственно пишет ровно один байт. Она, правда, почему-то принимает int, (вероятно, потому, что часто имплментация этого метода уходит в native, а в JVM нет байтов, только инты) но пишет только 8 нижних бит (т.е. можно просто кастовать byte к int и передавать в тот метод)

@danlapko danlapko merged commit cf7b399 into master Apr 27, 2018
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