pezra / exhal

Use HAL APIs with ease
MIT License
22 stars 15 forks source link

Allow re-use of param in `ExHal.Transcoder` #45

Closed d-mart closed 8 years ago

d-mart commented 8 years ago

When using a param name twice in a transcoder, like so:

defmodule HawaiiTranscoder do
  use ExHal.Transcoder

  defproperty "hello", param: :aloha
  defproperty "goodbye", param: :aloha
end

And then trying to encode a document:

HawaiiTranscoder.encode!(%{aloha: "aloha"})`

The resulting document lacks properties based off the same parameter after the first one:

%ExHal.Document{client: true, links: %{}, properties: %{"hello" => "aloha"}}

This comes from the @injectors and @extractors being built programmatically from the param name, then accumulated into the respective module variable. When looking through the @injectors or @extractors at run-time, the same (identical) one is found each time. This leads, in this case, to "hello" being added to the document once, then overwritten the second time. And the second property is not generated. Or something like that

The change herein adds some uniqueness to the injector_name and extractor_name. There are undoubtedly better ways to do this, but in general this approach works.

Alternatively, it might work to pass the property name into ExHal.Transcoder.interpret_opts and generate the injector names that way. Those should be unique as well.

pezra commented 8 years ago

What if we keep a counter module attribute a used that counter instead of a random string?

glassbead0 commented 8 years ago

πŸ’― ❗️ πŸ‘ πŸ‘ :shipit:

d-mart commented 8 years ago

I tried that (and maybe wrongly) but couldn't access @module_attributes inside of macro things

  defp interpret_opts(options, name) do
    @counter (@counter + 1)
    unique_string = @counter |> Integer.to_string
......
== Compilation error on file lib/exhal/transcoder.ex ==
** (ArgumentError) cannot set attribute @counter inside function/macro
    (elixir) lib/kernel.ex:2314: Kernel.do_at/4
    (elixir) expanding macro: Kernel.@/1
    lib/exhal/transcoder.ex:182: ExHal.Transcoder.interpret_opts/2
d-mart commented 8 years ago

:ets table at compile time?

ducks

glassbead0 commented 8 years ago

Any inklings of a merge sometime soon?

pezra commented 8 years ago

Released as 5.1.0