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

Best practice: How and where to put protocols and their implementation? #84

Open
falti opened this issue Apr 5, 2016 · 8 comments
Open

Comments

@falti
Copy link

falti commented Apr 5, 2016

I think it would help to have a common place to put protocol definitions and also implementations. It would be in particular interesting if protocols are implemented for existing modules.

For discussion:

definition:

file: lib/protocols/incredible.ex

implementation for a new modules within the module.

implementation for an existing module (e.g. Integer) in:

file: lib/protocols/impl/integer.ex

or

file: lib/protocols/integer_impl.ex

?

@ihildebrandt
Copy link
Contributor

I have been considering this question myself as well.

One way that I have been leaning would be to use a naming convention similar to the one used for categories in Obj-C. It would have the module name and protocol name separated by a plus: lib/integer+protocol.ex. This way you have the relevant information in the file name and it's easy to stick to the 'one module per file' convention already in place.

@Qqwy
Copy link
Contributor

Qqwy commented Jul 1, 2016

A good example of what Elixir's standard library does, is the Enumerable Protocol. I think we can use this as a good starting point.

Most protocols consist of three parts:

  1. The protocol specification.
  2. A module that is used to build more functions that internally use the few functions in the protocol specification. This part is optional, but often convenient.
  3. One or multiple implementations of the protocol for different data structures.

The Enumerable protocol in Elixir, as well as the public-facing Enum module built on top of it, as well as the implementations of Enumerable for List and Map are defined inside the enum.ex file.

Depending on how large your protocol will be, it might be nicer if it is split.

When defining a new implementation of a protocol for some data type, I think it makes the most sense to specify that implementation inside the module where you define that data type, e.g.

This has as advantage that you don't need the for: part in the call to defimpl.

defmodule BinaryTree do
  defstruct[:left, :right, :value]

  defimpl Enumerable do
    def reduce do
      # some implementation here
    end
  end

  # Other functions here
end

For built-in data types such as Lists, Maps or Strings, I think it makes the most sense if the protocol implementation is placed in the same file as the protocol specification itself.

When an implementation for a protocol is needed, but you don't have access to the module where the type is made because it is part of a dependency (or the core language), then it makes the most sense to me to put the implementation closely to the module where you're using the module implementation in.An example would be to define a currency_formatter implementation for the Decimal data type. Where to put this? Either in the file it is used, or in a file that is referenced by the file where it is used.

If you really want to put it in a separate file, I think that names like "#{datatype_name}_#{protocol_name}_impl.ex" are the most describing. so decimal_currency_formatter_impl.ex or something like that.

@michalmuskala
Copy link

The rule I usually follow:

  1. Define protocol in it's own file
  2. For stdlib types or types from dependencies define the implementations in the same file
  3. For custom types define the implementation in the same file that defines the struct.
  4. When both protocol and type are external, all bets are off. I never had to do this, but if I did, I would probably go with file named like the protocol, that hosts the implementations for external types - trying to make it similar to the situation from 2.

@corroded corroded added this to Backlog in Style guide Kanban board Mar 7, 2017
@corroded corroded moved this from Backlog to Needs Discussion / Clarification in Style guide Kanban board Mar 8, 2017
@corroded
Copy link
Collaborator

@michalmuskala @christopheradams @Qqwy - What would be the course of action here? Has there been an agreement as to what style to use?

I feel like for this one, it seems too broad and there may be too many ways to do this properly. It will be depending on your team to discuss?

@Qqwy
Copy link
Contributor

Qqwy commented Mar 13, 2017

Yes, there are multiple, equally valid (to be exact: which version is best depends on the precise situation) approaches. It might still be a good idea to list the multiple common approaches (which @michalmuskala summarized really well! 👍 ) side-by-side.

@michalmuskala
Copy link

Yeah, I don't think I have anything to add over what I already wrote here.

@corroded
Copy link
Collaborator

corroded commented Mar 17, 2017

@Qqwy @michalmuskala would anyone be keen to make an example of each use case? I don't have much experience with Protocols yet so not sure how to make examples like this one:

# not preferred
# bad protocol example here

# preferred
# not good example here

Or can we just list down @michalmuskala 's examples as is:

  • Define protocol in it's own file
  • For stdlib types or types from dependencies define the implementations in the same file
  • For custom types define the implementation in the same file that defines the struct.
  • When both protocol and type are external, all bets are off

And maybe a link to this thread for further reading?

@gbxl
Copy link

gbxl commented Mar 16, 2018

I'm curious as to how you guys test it? Do you defer the action to a Domain module function and test that module? Or do you test the protocol separately?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Style guide Kanban board
Needs Discussion / Clarification
Development

No branches or pull requests

7 participants