Skip to content

Conversation

@kravtsun
Copy link

@kravtsun kravtsun commented Mar 14, 2017

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.

10

* @throws SerializationException in case of IOException during deserialization
*/
void deserialize(InputStream in);
void deserialize(InputStream in) throws SerializationException;
Copy link

Choose a reason for hiding this comment

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

особого смысла в отдельном исключении нет, по желанию -- можно делать своё, можно IO


public StringSetImpl() {
root = null;
}
Copy link

Choose a reason for hiding this comment

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

этот конструктор можно убрать:

  • если поле ссылочного типа нигде не инициализируется (ни в конструкторе, ни в объявлении), то ему автоматически присвоится null
  • если в классе нет конструкторов, то будет создан конструктор по умолчанию (public, без параметров)

но можно и оставить, это по желанию

out.write(1);
} else {
out.write(0);
}
Copy link

Choose a reason for hiding this comment

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

можно тернарный оператор использовать out.write(b ? 1 : 0);

throw new SerializationException();
}
}

Copy link

Choose a reason for hiding this comment

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

про примитивы:

  • в этих методах можно было бросать IO например, а потом выше преобразовывать или оборачивать в SerializationException
  • можно было заиспользовать DataInput/DataOutputStream, тогда этих методов бы не было, но оборачивать в SerializationException всё равно пришлось бы

private static final int EMPTY_VERTEX_MAGIC = 0xDDCCBBAA;
private final Vertex[] next;
private boolean isTerminal;
private Vertex parent;
Copy link

Choose a reason for hiding this comment

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

final

private static class Vertex implements StreamSerializable {
private static final int CHAR_POWER = 2 * 26;
private static final int VERTEX_MAGIC = 0xAABBCCDD;
private static final int EMPTY_VERTEX_MAGIC = 0xDDCCBBAA;
Copy link

Choose a reason for hiding this comment

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

появление слова magic вносит неразбериху, и другому разработчику может показаться, что автор не очень понимает то, что сам написал)) в данном случае лучше использовать EMPTY_VERTEX_FLAG, NON_EMPTY_VERTEX_FLAG. Значения не очень важны, хоть 0 и 1.

}
}

// Question: constructor would be better?
Copy link

Choose a reason for hiding this comment

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

отсылаю к практике, когда обсуждались конструкторы и статик методы. мне текущий вариант больше нравится, он симметричен к методу выше

@@ -0,0 +1,74 @@
package ru.spbau.mit;
Copy link

Choose a reason for hiding this comment

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

тесты сильно усложнять не всегда имеет смысл, лучше написать побольше простых случаев, включая какие-то крайние

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