Skip to content
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

Add a bytes helper method to Index #182

Open
valencik opened this issue Mar 7, 2024 · 2 comments
Open

Add a bytes helper method to Index #182

valencik opened this issue Mar 7, 2024 · 2 comments
Labels
good first issue Good for newcomers

Comments

@valencik
Copy link
Contributor

valencik commented Mar 7, 2024

Currently we get the ByteVector of an Index with something like:

val indexBytes = MultiIndex.codec
  .encode(index)
  .map(_.bytes)
  .toEither
  .leftMap(err => new Throwable(err.message))

This feels like a lot of ceremony, let's add a bytes method to the Index trait that does this for us.

@valencik valencik added the good first issue Good for newcomers label Mar 23, 2024
@abinashlingank
Copy link

Hi! I'd like to work on this issue.

Solution:
I'll add a bytes method to the Index trait to simplify encoding and error handling. The new method will return the byte array as Either[Throwable, Array[Byte]], reducing the boilerplate code.

trait Index {
  def bytes: Either[Throwable, Array[Byte]] = {
    MultiIndex.codec.encode(this)
      .map(_.bytes)
      .toEither
      .leftMap(err => new Throwable(err.message))
  }
}

Now, we can call index.bytes to get the byte array directly.

Let me know if I can proceed!

@valencik
Copy link
Contributor Author

Hi @abinashlingank! 👋

That's close, but I think if you were to write that code you'd find that it won't compile.
The return type of your code isn't an Array[Byte], it is a ByteVector, which is actually what we want.

Additionally, because FrequencyIndex and PositionalIndex have different Codecs, this would have to be abstract on the Index trait.
Unless we made an abstract def codec: Codec[Index] on the Index and then the bytes helper method could be concrete and leverage that.
That would work.

I'm happy to have you proceed, it just might not be as straightforward as you've initially sketched out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants