PR’s welcome 
Because it is very likely the pagination library should also be just using functions instead of a plug. I would rather do query |> Repo.paginate(params) or query |> paginate(Repo) than the plug or decorator approaches.
I can’t say about other languages but the fact validations in Ecto are not decorators makes them extremely easy to compose. If you need conditional validations, dynamic parameters, etc, you can just write Elixir code instead of finding the proper decorator incantation.
When it comes to composition and flow-control, there is no decorator that is going to be simpler and more readable than pattern matching, with and the |> operator (for the simplest cases).
9 Likes
@josevalim thanks for your feedback. I’m afraid I don’t know enough yet to understand your reasoning. Here’s what we’ve built for our realtime api (using phoenix channels):
defparams status_react_params %{
status_id!: :integer,
reaction_type!: :string
}
@decorate validate(&status_react_params/1)
def status_react(~m{status_id, reaction_type}, socket) do
# ...
end
The goal here is to make sure that the incoming params are present and valid. In this example, if status_id is missing or a string, then we handle the error response appropriately and we’ve used an embedded_schema for determining if things are valid or not.
What we eventually validate as part of the db query may be an entirely different ecto schema. In my opinion, this is pretty readable. It’s easy to construct embedded_schemas and the logic for how to handle incorrect params is centralized in one place (the validate decorator).
How would our code be measurably more readable and composable than what we have now? Is there perhaps a scenario of how things can break down that maybe we haven’t thought about?
Here is the issue I see in your code. Imagine that the parameter you need to validate depend on some state in socket. Maybe it depends if the user is in their phone or if they are an admin. How are you going to apply this conditional validation? Maybe you need to add more sugar to defparams?Or maybe you need to define now two private functions with different behaviours that you’d call accordingly?
If this code was just Elixir code then the answer to how you would change any of this code is immediately obvious. You would handle it as any Elixir code. You would use pattern matching or conditionals, etc.
The type of conditional validation you are expressing can be done with functions and data types. There is no need for decorators or indirection:
@types %{
status_id: :integer,
reaction_type: :string
}
def status_react(params, socket) do
with {:ok, ~m{status_id, reaction_type}} <- validate_params(params, @types),
do: ...
end
You can use with or you can also use something like changesets, where you validate and annotate the types as you go.
5 Likes
Hey, @josevalim following up on your advice on not using decorators.
Is decorator a good implementation pattern when looking at it from a logging/instrumentation use-case?
We’re looking at implementing logging/instrumentation for function entry/exit calls across controllers/web-sockets/contexts and to be honest using a @decorator makes the job much easier.
Are there any other alternate options to go about such logging functionality in elixir apps that I’m missing.
The decorators have problem in form that they do not work nicely with multiple function definitions in case of pattern matching. Is this:
@decorate trace()
def sum([x | xs]), do: x + sum(xs)
def sum([]), do: 0
The same as this:
@decorate trace()
def sum([x | xs]), do: x + sum(xs)
@decorate trace()
def sum([]), do: 0
And this:
def sum([x | xs]), do: x + sum(xs)
@decorate trace()
def sum([]), do: 0
If yes, then how will you detect when to decorate function? If not then how it will not be confusing?
In case of logging and instrumentation I think the best solution will be just define function like:
def trace(name, func) do
ctx = start_trace(name)
try do
func.()
after
end_trace(ctx)
end
end
And use it like that:
def my_traceable_func() do
trace(:my_traceable_func, fn ->
# do thing
end)
end
Alternatively you can use macro to extract some more information at the compile time.
