-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create CrcBuilder
#316
Create CrcBuilder
#316
Conversation
@@ -49,6 +49,13 @@ object crc { | |||
lazy val crc32c: BitVector => BitVector = | |||
int32(0x1edc6f41, 0xffffffff, true, true, 0xffffffff).andThen(i => BitVector.fromInt(i)) | |||
|
|||
/** An immutable "builder" to incrementally compute a CRC. | |||
*/ | |||
sealed trait CrcBuilder[Out] { |
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 don't hold this opinion strongly but wdyt of using the names from collection library?
- rename
update
to+=
- rename
output
toresult
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.
Eh on second thought, I don't like that suggestion as this is immutable.
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.
for sure, these definitely need bike-shedding :)
- 👍
output
toresult
update
to+=
: maybe as an alias? it's a bit confusing since we're not exactly "adding", is there some other symbol that could be more appropriate? Also, since it's immutable, I think we'd want+
so that+=
works if they store it in avar
.
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.
Oops, just saw your followup comment. Yep!
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.
Btw, should update
be updated
? I think that's how they do immutable structures?
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.
👍 for updated
and result
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.
Done and done!
As discussed in typelevel/fs2#2606. This was pretty straightforward, just code shuffling.