-
Notifications
You must be signed in to change notification settings - Fork 116
I1132 #362
base: main
Are you sure you want to change the base?
Conversation
type ModifierTypeId = ModifierTypeId.Type | ||
|
||
type VersionTag = VersionTag.Type | ||
type VersionTag = util.ModifierId |
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.
I think this is confusing reuse, which complicates things.
So I suggest to declare
object VersionTag extends TaggedType[util.ModifierId]
So we don't accidentally confuse two types.
In particular, is it always 32 long hash?
Also ScalaDoc is missing, so these assumptions should be described in ScalaDoc.
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.
I think that the main source of confusion is that there are many different types which are in0turn just wrappers around Array[Byte]:
ModifierId, VersionTag, EncodedTokenId, EncodedBoxId, ByteArrayWrapper
From my point of view I leave just one implementation (my choice is ByteArrayWrapper, because it most clear term) and may be left other types as aliases.
Concerning size of byte array: is it always 32 bytes or can have arbitrary length, I also do not know whether all hashes (keys) used in Ergo are 32 bytes long, but definitely it is not true for generic ByteArrayWrapper. Also my choice is to not make this assumption in ByteArrayWrapper implementation. If you think that speed of hash function is so critical, we can add just one extra if:
override def hashCode: Int = {
val bytes = hashBytes
if (bytes.size == 32) {
hashFromBytes(bytes(28), bytes(29), bytes(30), bytes(31))
} else {
java.util.Arrays.hashCode(bytes)
}
}
No description provided.