[WIP] DO NOT MERGE - Initial implementation#5
Conversation
|
|
||
| private transient WeakReference<String> utf8String; | ||
|
|
||
| public static final Bytes EMPTY = new Bytes(new byte[0]); |
There was a problem hiding this comment.
Would probably be cleaner to hide this and have a no-arg of() method.
There was a problem hiding this comment.
I thought about an empty() method, which is the same thing, but a little more intuitive. However, I didn't see much difference between a method and a constant. Are there benefits to the method version over the constant?
There was a problem hiding this comment.
I think the main benefit of of() is consistency
There was a problem hiding this comment.
I think "of" is the wrong word in this case. I think good fluent interfaces mimic natural language sentences, and an "of" preposition with a blank instead of a direct object to follow would be less "fluent". I think it's probably better to sacrifice consistency by changing the method name in fluent APIs in order to favor readability.
What do you think of empty()?
There was a problem hiding this comment.
empty() is ok w/ me, but I still prefer of(). I have seen both used. I have seen guava use of() in classes like ImmutableMap. Can't remember where I saw empty() used.
| * | ||
| * @since 1.0.0 | ||
| */ | ||
| public interface ByteSequence extends Iterable<Byte> { |
There was a problem hiding this comment.
Why make this extend Iterable<Byte>?
There was a problem hiding this comment.
I think I was following the CharSequence pattern of being Iterable over Character.
| * | ||
| * @since 1.0.0 | ||
| */ | ||
| abstract class AbstractByteSequence implements ByteSequence { |
There was a problem hiding this comment.
Could provide default implementations of contentEquals(byte[]) and compareTo(byte[]).
| /** | ||
| * Returns a byte array containing a copy of the bytes | ||
| */ | ||
| public byte[] toArray() { |
There was a problem hiding this comment.
Wondering if this method should be in ByteSequence.
There was a problem hiding this comment.
Oh, yeah, that makes sense.
| * @param index the position within the sequence to retrieve | ||
| * @return the byte at the specified index | ||
| */ | ||
| byte byteAt(int index); |
There was a problem hiding this comment.
Not sure if this is a good thing to do, but its something I thought of. Could make index of type long.
| * | ||
| * @return the length of the sequence | ||
| */ | ||
| int length(); |
There was a problem hiding this comment.
This could possibly be long. Again, not sure if this is a good idea.
|
|
||
| private int hashCode = 0; | ||
|
|
||
| public Bytes() { |
There was a problem hiding this comment.
If all constructors are package private then, the class is effectively final. So we can drop final from the class definition. This allows us the flexibility to subclass Bytes for internal implementations that have very different behaviors. These internal package private subclasses could only be created by static methods. This is the approach taken by protobuf.
Update Javadoc for Bytes.toString() method to clarify the strategy for constructing the Java String is to decode the Bytes with UTF-8.
3 participants
This is a work in progress. It is not quite ready to merge. I want to make it available for review/feedback before I make any more progress, to make sure it's headed in a good direction.