Phoenix v1.3.0-rc.0 released

Thanks for the feedback! Answers inline.

21 functions is not a huge API surface area. I can understand the concern of God Modules, but as previous posts have shown, it’s difficult to make suggestions if a module is doing too much without concrete use cases. Also note that just because you have 3 entities in your context, it does not necessarily mean you’ll have the same conventional functions for all of them. Sometimes you will, sometimes you won’t. For example, imagine a a bank system with a Deposit, an Account, and a AccountHolder. Under this case, you likely wouldn’t expose the crud functions for deposits at all, instead delegating the details of that deeper into the system. That’s not to say you won’t end up with > 21 functions. As José has said, it’s easier to start with the parts of your app in the same place, then see where they can be separated than trying to predict the future up front. If these parts of your app are necessarily coupled and depend on one another, grouping together their functionality is not a problem. If you have specific examples we can try to say more.

There is duplication here if all your entities are using ecto, but I don’t feel this is a good candidate for code injection. use GenServer is injecting code to satisfy a behaviour with known functionality. The point of your contexts is that they are the boundary to the internal details of listing users/roles/permissions. If your user permissions are stored in ets, but the users and roles in Postgres, then you can’t just inject code. I also would prefer to see an explicit call when viewing the module. If you want to offload the query generation that you handle the same way, then I would do something like this:

def list_users(opts \\ []) do
  opts
  |> QueryBuilder.apply_opts(User)
  |> Repo.all()
end

def list_permissions(user, opts) do
  from(p in assoc(user, :permissions))
  |> QueryBuilder.apply_opts(opts)
  |> Repo.all()
end

Notice how even using your user and permission example, our “duplicated” api for listing resources already contains some differences. For example, list/create/update/delete_permissions is going to take the user as an argument, where the generated functions won’t have such scoping. I don’t feel code injection is best here, but there are still ways to offload shared querying by delegating outside, as QueryBuilder in my example that would know how to build an Ecto query given some options.

Changests can be for any source, and I usually go the other way and collocate them close the user input or changes I am modeling. That said, there is nothing stopping you from collocating changeset functions with the schema. It’s a valid option if that suits you, but since my boundaries expose my Schema structs publicly, I like to keep private things like changeset building out of the module.

This is still a good approach, but since Ecto achieved parallel database transactions, this design is no longer an essential part of your testing strategy. I actually find it easier to write and maintain tests that simply hit the database internally as needed than those that go through hoops to manage or stub db calls.

The first part of this sentence is absolutely true – the new context generators will result in more code than throwing a Repo.insert in the controller, but as far as maintenance is concerned seeing the LOC that the new generators build vs the old ones is not a good maintenance indicator. For example, every time you duplicate Repo.insert / Ecto.build_assoc/ etc calls in multiple controllers, channels, other modules, etc, you are placing a future burden on yourself. Compound this over a few years, and it will be a massive maintenance cost. Imagine needing to offload persistence of parts of the system to another store, or another app entirely? Now you have to refactor every part of the system that this “simpler” code touched.

Every decision we make in software is a trade-off, but I feel the current approach maintenance wise is an absolute win, with the potential for slightly more effort up front. We feel that effort is going to be worth it :slight_smile:

9 Likes