svarlet

svarlet

Modules don't compose well

Hi all,

I currently have this thought that relying solely on modules in Elixir hinders the writing of polymorphic code. Let me explain my thought process with an example.

Let’s imagine a shop project, using phoenix and ecto, and let’s roughly implement a route that creates a purchasable product. This has to follow some rules defined by the Sales and Strategy teams. For example, the name must be unique, there must be a sell by date no later than 3 months after creation, it has to be approved by the CEO before customers can purchase it, etc.

In the router:

post "/purchasables", PurchasablesController, :create

In the controller

def create(conn, params) do
  purchasable_creation_request = %PurchasableCreationRequest{
                                      name: params.name,
                                      approved: false,
                                      created_at: Timex.now()}
  case BusinessRules.Purchasables.create(purchasable_creation_request, PurchasablesGateway) do
    {:ok, purchasable} ->
      conn
      |> put_flash(:info, "Success")
      |> assign(purchasable, purchasable)
      |> render(:show)
    {:error, reason} ->
      conn
      |> put_flash(:error, "Creation failed because #{reason}")
      |> render(:create)
  end
end

Here we have the controller convert the http request into something the business rule can work with. The dependency and flow of control only go in one direction. While we have successfully split all the code into 2 modules, they are still tightly coupled.

If I look at the source code dependencies and the flow of control, they both go in the same direction:

  • The PurchasablesController uses BusinessRules.Purchsables.create/2
  • The PurchasablesController depends on BusinessRules.Purchsables.create/2

Concrete things use and depend on concrete things.

At the same time, it looks like BusinessRules.Purchasables.create/2 uses a gateway for purchasables but does not depend on a concrete one.

In a polymorphic setup, we would often observe that. The flow of control opposes the source code dependencies: it’s not because ModuleA uses Ecto that it has to depend on Ecto.

Interestingly this is very palpable in tests. Howvever, there is a variety of opposing opinions in this forum held for or against testing, for or against TDD, for or against mocking, … so I hope this topic is not going to deviate towards any of them.

Back to the example, if we wanted more polymorphism, we could loosen the dependency between the controller and the business rule:

In the router

post "/purchasables", PurchasablesController, :create,
  private: %{
    business_rule: BusinessRules.Purchasables
  }

In the controller

def create(conn, params) do
  purchasable_creation_request = %PurchasableCreationRequest{
                                        name: params.name,
                                        approved: false,
                                        created_at: Timex.now()}
  case conn.private.business_rule.create(purchasable_creation_request, PurchasablesGateway) do
    {:ok, purchasable} ->
      conn
      |> put_flash(:info, "Success")
      |> assign(purchasable, purchasable)
      |> render(:show)
    {:error, reason} ->
      conn
      |> put_flash(:error, "Creation failed because #{reason}")
      |> render(:create)
  end
end

So what happened here? While the controller needs a business rule, it doesn’t need to know which one it uses. I found this morning that the plug router now allows to pass stuff via conn.private to a plug (controllers are plugs). I thought it would be a better place to set the concrete business rule used by the controller. That makes my controller code easier to test as I can test the behaviours of the controller without involving the rest of the app like the business rule and the DB gateway.

Though it doesn’t feel quite right yet. Using the router for that feels like misplaced responsibility. I’d like to find a way to compose all the things together in a different way:

  1. make/create/start a purchasable gateway
  2. make/create/start a business rule with that gateway
  3. make/create/start a controller with the business rule
  4. associate the controller with a http route

Some might say it’s a OOP mindset to try to compose things together, but it’s not reserved to OOP, for example in F#: Integration Testing composed functions

Today, the community seems to prefer the use of Application config with module attributes everywhere there is a need for indirection.

For example, in the controller:

defmodule PurchasablesController do
  use Phoenix.Web, :controller

  @business_rule Application.get_env(:my_shop_app, :purchasable_creation_rule)

  def create(conn, params) do
    purchasable_creation_request = %PurchasableCreationRequest{
                                        name: params.name,
                                        approved: false,
                                        created_at: Timex.now()}
    case @business_rule.create(purchasable_creation_request, PurchasablesGateway) do
      {:ok, purchasable} ->
        conn
        |> put_flash(:info, "Success")
        |> assign(purchasable, purchasable)
        |> render(:show)
      {:error, reason} ->
        conn
        |> put_flash(:error, "Creation failed because #{reason}")
        |> render(:create)
    end
  end
end

I think it’s dangerous because the tests often can’t run safely and concurrently, I tend to define all tests as async: true by default. It’s muscular memory at this point. Am I willing to trade test execution speed / time to feedback for this? No. I found using the Mox library, a good, safe and fast fix though.

I wonder if we could make these dependencies more explicit though. Whether with the router or with application config, the dependencies are rather implicit, with Application being the worse.

Let’s look back at our last version of the example (no matter which approach between the router or app config, you pick). I noticed that the business rule is now polymorphic: I can easily pick a different one. Yet the controller is still making a decision about which concrete gateway the business rule is using. I don’t think that’s the job of the controller and so the code should look like this:

In the controller:

defmodule PurchasablesController do
  use Phoenix.Web, :controller

  @business_rule Application.get_env(:my_shop_app, :purchasable_creation_rule)

  def create(conn, params) do
    purchasable_creation_request = %PurchasableCreationRequest{
                                        name: params.name,
                                        approved: false,
                                        created_at: Timex.now()}
    # Removed the gateway in the next line
    case @business_rule.create(purchasable_creation_request) do
      {:ok, purchasable} ->
        conn
        |> put_flash(:info, "Success")
        |> assign(purchasable, purchasable)
        |> render(:show)
      {:error, reason} ->
        conn
        |> put_flash(:error, "Creation failed because #{reason}")
        |> render(:create)
    end
  end
end

We’ve just decoupled things a bit more. Our business rule still needs a gateway though. Sure, we could once again rely on Application config to make the business rule fetch its gateway at compile time into a new module attribute. Things are getting more and more implicit now, it’s spreading quickly. Perhaps we could combine this solution with the router+option solution:

post "/purchasables", PurchasablesController, :create,
  private: %{
    business_rule: BusinessRules.Purchasables,
    gateway: PurchasablesGateway
  }

and revert the change made to the controller to

defmodule PurchasablesController do
  use Phoenix.Web, :controller

  @business_rule Application.get_env(:my_shop_app, :purchasable_creation_rule)

  def create(conn, params) do
    purchasable_creation_request = %PurchasableCreationRequest{
                                        name: params.name,
                                        approved: false,
                                        created_at: Timex.now()}
    case @business_rule.create(purchasable_creation_request, conn.private.gateway) do
      {:ok, purchasable} ->
        conn
        |> put_flash(:info, "Success")
        |> assign(purchasable, purchasable)
        |> render(:show)
      {:error, reason} ->
        conn
        |> put_flash(:error, "Creation failed because #{reason}")
        |> render(:create)
    end
  end
end

Well, now we have combined 2 different solution so it’s more complex. Worse, the controller knows way too much about lots of little details to make things work, it is nosy.

What if we look at the other solution using the router options only? A solution is to define a module which wraps the actual business rule and decide what gateway should be used. That’s like decorators in the OOP world.

#Can this scale into the composition root described by ploeh in the blog shared above?
defmodule PurchasableCreationRuleWithBatteriesIncluded do
  def create(purchasable_creation_request) do
    BusinessRules.Purchasables.create(purchasable_creation_request, PurchasabesGateway)
  end
end

In the router:

post "/purchasables", PurchasablesController, :create,
  private: %{
    business_rule: PurchasableCreationRuleWithBatteriesIncluded
  }

The controller:

defmodule PurchasablesController do
  use Phoenix.Web, :controller

  def create(conn, params) do
    purchasable_creation_request = %PurchasableCreationRequest{
                                        name: params.name,
                                        approved: false,
                                        created_at: Timex.now()}
    # the concrete business rule is set in the router and has the desired gateway baked in
    case conn.private.business_rule.purchasable_creation_request) do
      {:ok, purchasable} ->
        conn
        |> put_flash(:info, "Success")
        |> assign(purchasable, purchasable)
        |> render(:show)
      {:error, reason} ->
        conn
        |> put_flash(:error, "Creation failed because #{reason}")
        |> render(:create)
    end
  end
end

That is a solution I found while writing this up. Disclaimer: I haven’t actually tried it yet, but I still don’t feel very excited about it. It all feels like doing functional programming in older versions of Java: instead of composing functions, we are finding workarounds to compose modules, and that’s not … elegant.

Thoughts? Opinions?

Most Liked

josevalim

josevalim

Creator of Elixir

I am not going to discuss the overall architecture ideas here but I would like to talk about the “Modules don’t compose well” bit, in particular this one:

Modules in Elixir provide the same expressive power as functions. From the perspective of functional programming, they are the same, modeled by the same principles, and the same power for composition. Maybe one is more verbose than the other, and that may be what “compose well” means in this context, but they are not different in the ability to compose.

If you have two functions, how can you compose them? We can write the compose function like this:

defmodule Foo do
  def add_2(x), do: x + 2
  def mult_2(x), do: x * 2
  def compose(fun1, fun2), do: fn x -> fun2.(fun1.(x)) end
end

And now:

iex> Foo.compose(&Foo.add_2/1, &Foo.mult_2/1).(13)
30

How can we compose modules? In similar way. Let’s assume each module has a contract, it has to implement call:

defmodule Add2 do
  def call(x), do: x + 2
end

defmodule Mult2 do
  def call(x), do: x * 2
end

defmodule Compose do
  def compose(mod1, mod2) do
    name = Module.concat(mod1, mod2)

    defmodule name do
      @mod1 mod1
      @mod2 mod2

      def call(x) do
        @mod2.call(@mod1.call(x))
      end
    end

    name
  end
end

And now:

iex> Compose.compose(Add2, Mult2).call(13)
30

The difference between the approaches is in their cost: defining the modules are much more expensive. But this is a runtime property. Plus, it can also be made cheaper by having each “module call” be represented by a tuple {Mod, arg}, and then you can compose without defining modules dynamically. Namely, the benefit of functions is that you can close over the current environment (closure) and by using a tuple you can emulate the same with modules.

in fact, one can say the advantage of a module is that you can define multiple functions and compose over multiple functions at once, but this is also possible with anonymous functions. You just need to add a new argument to the function signifying the operation you want to do. From a functional programming perspective, there isn’t much difference between those two:

defmodule Calculator do
  def add(a, b), do: a + b
  def mult(a, b), do: a * b
end

calculator = fn
  :add, a, b -> a + b
  :mult, a, b -> a * b
end

Protocols also provide the same sort of composition as anonymous functions, with the benefit they are open and can be defined/implemented at any time. In fact, a protocol may be a nice solution for your problem.

Anyway, the reason why I am saying this is not to say “you are wrong”, but maybe it can help you put a finger on what you don’t like about the current code.

I am not sure I follow the point here. Plug pipelines can be defined completely dynamically. For example:

  [
    {Plug1, opts},
    {Plug2, opts},
    {Plug3, opts},
  ]

And then to execute it:

plugs
|> Enum.reduce_while(conn, fn {plug, init}, conn ->
  case plug.call(conn, plug.init(init)) do
    %{halted: true} = conn -> {:halt, conn}
    %{} = conn -> {:cont, conn}
  end
end
|> elem(1)

This can also be meta-programmed and compiled into a module when the app starts for performance. But in a nutshell, you can always move the Plug to the runtime. However, I would avoid doing this, because understanding what my pipeline is actually doing becomes much harder. I would indeed prefer to keep them static and read the state from elsewhere.

And this is somewhat my concerns with the proposed code in the thread: the code may be less coupled but we lose a lot in clarity. This could be beneficial if the controller ultimately become a dumb layer (imagine that multiple gateways and business rules are served by the same controller) but I personally wouldn’t do it for individual cases.

This is not something Ace solves either because if you want to make decisions based on the user, which you would most likely do, by the time you know exactly which user you have, you are already too deep into the request life-cycle (and therefore inside both Ace and Plug requests). Futhermore, both Ace and Plug are module based contracts and both allow you pass arguments when building the supervision tree.

michalmuskala

michalmuskala

I don’t mean plugs at all in here. I’m talking about processes - the process that is started for the request happens somewhere completely in the internals of the library and you have no access to that. The regular way of setting up state for services through start_link and init is not available.

What I’d like to see would be to have something like:

# in application.ex
children = [..., {MyApp.Endpoint, some_initial_options}, ...]

# in MyApp.Endpoint
def init_conn(conn, those_options) do
  # set up initial state for the Conn struct that is later used by this request
  # basically - the same as init for a gen_server
end

This would allow handling plug request processes as any other processes in the system - right now they are somewhat special and separated from the supervision tree in that there’s no way to pass state into them. The whole state has to be either provided statically (in some module somewhere) or through mutable memory (an ets table or application env).

michalmuskala

michalmuskala

I agree with this sooo much. I think the problem lies exactly in the fact that we can’t inject state into the Plug connections from the regular supervision structure - for any other processes we have start_link and the init callback or other mechanisms, where we can set-up the state for the process - including dependencies, but we entirely lack that possibility with plug. For some reason Plug tries to enforce completely stateless processes.

Last Post!

Crowdhailer

Crowdhailer

Creator of Raxx

I haven’t read the details of everything in this thread but in summary Raxx has a few opinions about these things.

Configuration is passed in at boot time of a server, that handles the problem of having to do stuff on every request, but it is entirely up to the developer what shape of stuff is in config. I normally have a MyApp.Config struct and that module has setup function that will create a struct by whatever means necessary (e.g. pulling env variables).

For testing I start a separate server for each test so have the option to override any config value for the duration of the test.

I haver personally never made business logic configurable, but most side effect things that the business logic needs are passed in, e.g. sending emails, calling third party apis, db access (sometimes).

Normally I don’t test at the business logic/interaction layer but instead I have a lot of tests that create requests, pass them to the endpoint and assert on the response.

def handle_request(%{method: :GET, path: []}, config) do
  MyApp.BusinessAction.create(data_from_request, config.mailer)
  response(:ok)
end

Hope that’s helpful. It might be time for me to write a blog post about some of these things, but on the other hand I don’t feel my web layer needs to prescribe anything about how the various pieces of the whole application are composed together.

Where Next?

Popular in Discussions Top

AstonJ
Are there any Elixir or Erlang libraries that help with this? I’ve been thinking how streaming services like twitch have exploded recentl...
New
CharlesO
Erlang :list.nth simple, but 1 - based nth(1, [H|_]) -> H; nth(N, [_|T]) when N > 1 -> nth(N - 1, T). Elixir Enum.at … coo...
New
MarioFlach
Hello, I want to share a project I’ve been working on for a while: https://github.com/almightycouch/gitgud Background Some time ago I ...
New
jer
I’ve been using umbrellas for a while, and generally started off (on greenfield projects at least) by isolating subapps based on clearly ...
New
klo
Got a question about when to concat vs. prepending items to list then reversing to achieve appending. So i know lists boil down to [1 | ...
New
marciol
Please, let me know if this kind of discussion already took place in another topic . Hi all, how do you consider if is better to build ...
New
cvkmohan
The upcoming Phoenix 1.6 release looks very interesting. Became a habit to watch the commits - and - what they are bringing in. phx.gen...
New

Other popular topics Top

openscript
Hello! Sorry for this astonishing simple question, but I’m really stuck. I try to set up the intellij-elixir plugin, but I don’t know ho...
New
Darmani72
If I have a post route which an argument: post /my_post_route/:my_param1, MyController.my_post_handler How would get the post params ...
New
gshaw
What is the idiomatic way of matching for not nil in Elixir? E.g., First way: defp halt_if_not_signed_in(conn, signed_in_account) when...
New
axelson
This post is a wiki (feel free to hit the edit button near the bottom right of this post to add your own changes!) This post collects co...
239 49084 226
New
gausby
I asked this very same question on twitter and got some interesting feedback, but I thought it would be a good question to ask here as we...
1207 40042 209
New
senggen
Erlang/OTP 25 [erts-13.2.2] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] 15:22:35.803 [error] gen_event {lager_file_backend...
New

We're in Beta

About us Mission Statement