pavlos / gen_fsm

Elixir wrapper around OTP's gen_fsm
Other
40 stars 3 forks source link

Converted some wrappers to delegates #8

Closed gausby closed 8 years ago

gausby commented 8 years ago

Fixes #7

Notice that future versions of defdelegate might support default values, so at that point we could do away with the

  defdelegate [sync_send_event(fsm, event), sync_send_event(fsm, event, timeout)], to: :gen_fsm

and instead write

  defdelegate sync_send_event(fsm, event, timeout \\ 5000), to: :gen_fsm

I've asked around and this might be added in Elixir 1.3

pavlos commented 8 years ago

Since defdelegate doesn't support defaults yet, can we only use it for functions that don't have defaults?

Also, won't we lose the typespecs, or is there a way to avoid that?

gausby commented 8 years ago

Dialyzer can infer the types, so it should be all good, no worries.

In the cases that had defaults I just added the two versions with arities to the list of delegates. It is not that big of a deal, but we can consider putting them together as one when Elixir support defaults in delegates—or even then consider keeping them as is, because right now we support more versions of Elixir.

I didn't delegate the stop/3. I have been thinking about that, because I think there is only stop/1 and stop/3; as I read the Erlang :gen_fsm docs you have to define a timeout if you give a reason to stopping—as always, I could be wrong about all this :)

pavlos commented 8 years ago

Oh, I should have been more clear - dialyzer can indeed infer types, but can ex_doc? It's always nice when docs show types. Also, can you use @doc with delegated functions?

gausby commented 8 years ago

Well, I guess we can do both, this is how they do it in the Elixir Core Maps implementation https://github.com/elixir-lang/elixir/blob/e86e44da3425cf8d66cd698187c655739a2e447b/lib/elixir/lib/map.ex#L15-L25

From now on all arguments shall be backed by links to the Elixir core! I can update the pull request later to follow this style. I need to focus on another project for at couple of hours :)

pavlos commented 8 years ago

Oh sweet, i like that approach - idiomatic and concise, without sacrificing documentation :+1:

gausby commented 8 years ago

There. What do you think about that?

Sorry I dropped the ball on this for a couple of days ^_^

The documentation is lifted from the Erlang docs and Elixir-ified…if that is a word.

gausby commented 8 years ago

I've seen a mistake. Should it be GenFSM or GenFsm ?

Edit: Picked sides, I just changed it to GenFSM in the docs.

pavlos commented 8 years ago

this is looks great, I think I will:

Ship It