I recently had the opportunity to continue to level up my Elixir skillset by working on an internal API with a few of my collegues. So far, we’ve been focused on improving test coverage and cleaning up some aspects of the application design. We’ve been working together in numerous mob and pair programming sessions as well as reviewing each other’s PRs of independently completed work.
Instead of focusing on just writing code that works, many of the discussions in our pairing sessions and reviews have been around writing code that works while also improving the overall development experience for all engineers working with the codebase. I’d like to share a few of the suggestions that came up that I found valuable to keep in mind for the next time I am writing or reviewing code.
1. Don’t rely on “truthiness” of nil in tests
I had an assertion in a test that was confirming the absence of an entity in a list.
refute Enum.find(items, &(&1.name == absent_item.name))
This will work fine since Enum.find/3 returns an entity or nil, but it relies on the “truthiness” of nil. A cleaner approach would use Enum.any?/2 and explicitly rely on the returned boolean values.
refute Enum.any?(items, &(&1.name == absent_item.name))
2. Be mindful of implicit behavior and provide flexibility where it make sense
In the below example we can see that whenever list_items/0 is called, the :labels relationship is always preloaded.
Do we need the API to be rigid or should we add more flexibility? Should the caller know what relationships exist that can be preloaded? In this case we’re fulfilling a JSON:API spec. Since JSON:API let’s us declare which relationships get loaded, it makes sense for the caller to be able to explicitly define those.
Instead of implicitly adding preloads to a query within a Context, we could add a preloads option that will allow the consumers of the Context to explicitly list the preloads that they are concerned with. There’s no hidden behavior being abstracted away within the items context.
Neither approach is always right or wrong; what’s important is taking the time to ask questions to determine which implementation works best for your scenario.
# items.ex
def list_items do
Item
|> preload([:labels])
|> Repo.all()
end
# items.ex
def list_items(opts \\ []) do
Item
|> preload(Keyword.get(opts, :preloads, []))
|> Repo.all()
end
# item_controller.ex
Items.list_items(preloads: [:labels])
3. Happy path readability
Unless it is necessitated by the casing logic, put the error case at the bottom for better happy-path readability.
|> case do
{:error, reason} -> {:error, reason}
{:ok, %{status_code: 200}} -> {:ok, "success"}
{:ok, %{status_code: 400, body: body}} -> handle_error(body)
end
|> case do
{:ok, %{status_code: 200}} -> {:ok, "success"}
{:ok, %{status_code: 400, body: body}} -> handle_error(body)
{:error, reason} -> {:error, reason}
end
4. Alias the module to be tested (vs. importing)
When reading this first example, default_params/1 looks like a local test function. In reality, it is a function on the ParamModule that we are testing. By changing the import to an alias, we can make the intent of our test clearer and explicitly call the module to be tested.
# param_module_test.exs
import FunProject.ParamModule
test "returns defaults when params is empty" do
assert %{is_default?: true} == default_params(%{})
end
# param_module_test.exs
alias FunProject.ParamModule
test "returns defaults when params is empty" do
assert %{is_default?: true} == ParamModule.default_params(%{})
end
Since the introduction of compilation tracers in Elixir v1.10 it is possible to rewrite imports to aliases.
5. Alias full module name for improved global search
Instead of only aliasing down to FunProject.HTTPClient and referencing HTTPClient.Mock for our mock, we should alias the full module name down to FunProject.HTTPClient.Mock. Now if future developers were to come accross FunProject.HTTPClient.Mock in our config/test.exs they would be able to do a text search and see all of the places where it is used.
alias FunProject.HTTPClient
test "works" do
expect(HTTPClient.Mock, :request, fn -> {:ok, %{}} end)
end
alias FunProject.HTTPClient.Mock, as: HTTPClientMock
test "works" do
expect(HTTPClientMock, :request, fn -> {:ok, %{}} end)
end
My takeaway from all of these suggestions? Take the time to ask yourself what assumptions are being made and if they can be made more explicit. By writing implementations that focus on making the intentions of code clear and explicit, we can lower the cognitive load for any future engineers working on the codebase.
DockYard is a digital product agency offering custom software, mobile, and web application development consulting. We provide exceptional professional services in strategy, user experience, design, and full stack engineering using Ember.js, React.js, Ruby, and Elixir. With a nationwide staff, we’ve got consultants in key markets across the United States, including San Francisco, Los Angeles, Denver, Chicago, Austin, New York, and Boston.
