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

Don't save order and eq in the data structure? #15

Open
turion opened this issue Nov 2, 2022 · 7 comments
Open

Don't save order and eq in the data structure? #15

turion opened this issue Nov 2, 2022 · 7 comments

Comments

@turion
Copy link

turion commented Nov 2, 2022

When one would like to serialize a ClosedIntervals to the disk and/or via JSON, it's problematic to have the functions order and eq as part of the data structure. Maybe it's better to define a protocol and dispatch it on the data contained in the tree?

@turion
Copy link
Author

turion commented Nov 2, 2022

Also see #9 for a potential simplification of such a protocol.

@evnu
Copy link
Owner

evnu commented Nov 3, 2022

I don't understand what such a protocol would look like. Can you give an example?

@turion
Copy link
Author

turion commented Nov 4, 2022

I believe for example like this:

defprotocol Compare do
  @spec compare(t, t) :: :eq | :lt | :gt
  def compare(lhs, rhs)
end

defimpl Compare, for: Integer do
  def compare(lhs, rhs) when is_integer(lhs) and is_integer(rhs) do
    cond do
      lhs > rhs -> :gt
      lhs == rhs -> :eq
      lhs < rhs -> :lt
    end
  end
end

test "compare works on integers" do
  assert Compare.compare(2, 3) = :lt
end

Any data type that ClosedIntervals would contain needs to implement this protocol instead of attaching the functions eq and order to the structure.

See https://elixir-lang.org/getting-started/protocols.html

@evnu
Copy link
Owner

evnu commented Nov 4, 2022

I don't see how this simplifies serializing a closed interval over how it is currently done. To serialize a closed interval, the to_list/1 function can be used. This does not store eq or ord, so they have to be provided when deserializing with from/2. But that can be wrapped in a helper function. What advantage does a protocol give over this approach?

@turion
Copy link
Author

turion commented Nov 7, 2022

If we would use the protocol, we could directly use e.g. Jason.encode! without having to preprocess with an extra function. That is simpler. One has to do no extra work.

@evnu
Copy link
Owner

evnu commented Nov 9, 2022

One has to do no extra work.

But you will have to pass the order and eq functions around all the time as well when using the API of closed_intervals. So there is still extra work. I don't understand yet how you imagine the API of the project to be when using protocols, so I am leaning towards closing this ticket.

If your use-case is to enable Jason.encode!, we can simply add a custom encoder for the ClosedIntervals struct as part of this project. Decoding from json would still be up to the user, but that is ok, as Jason does not have a protocol for decoding anyways.

@evnu
Copy link
Owner

evnu commented Nov 14, 2022

I am not convinced that this suggestion improves upon the existing API. Maybe we need a PR to investigate the idea further. I think a Protocol will not be needed, but we could attempt to pull out the functions and force the user to always provide the functions when calling any API function.

This was referenced Nov 14, 2022
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

No branches or pull requests

2 participants