Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

ElixirNitpicks

wiki.alopex.li

Post reply on HN
Print view View original

51–60 of 60 posts

Re: ElixirNitpicks

#51

Post by josevalim »

I enjoyed reading the previous articles, so I was excited to read this one too. I really appreciate this style of feedback. Comments below.

-------

ERROR HANDLING

In my opinion, the "with {:is_email, true}" style is missing the forest for the trees. The whole point of "with" is to match on a consistent result. If you need to distinguish individual clauses, then you should either use case/cond or normalize the result types, in the same way you would do in Rust. So in your case I'd add two functions: "validate_email" and "check_email_availability" that returns ":ok" or "{:error, reason}". Then you end-up with:

    with :ok
We give similar examples in our anti-patterns docs: https://hexdocs.pm/elixir/main/code-anti-patterns.html#compl...

-------

STATE MANAGEMENT

Generally agreed. Just one nit:

> Sure the state is all encapsulated into processes, but then those processes are hidden behind an abstraction layer that makes them invisible, so really you’re just touching global variables.

They are not invisible. You can use Observer, the Phoenix Live Dashboard, and many other tools to traverse, explore, and navigate the supervision tree, processes, and see where the state is!

-------

IMPORTS

Agreed. We had several discussions on how to improve this but nothing satisfactory. Maybe it is time for another tango.

-------

MIXED MESSAGES

I'd say we actually do a good job on the official docs on the topics that are directly related to Elixir:

* On umbrella projects, the official guide discusses trade-offs: https://hexdocs.pm/elixir/dependencies-and-umbrella-projects...

* Live upgrades are covered in our release docs: https://hexdocs.pm/mix/Mix.Tasks.Release.html#module-hot-cod...

* On macros: https://hexdocs.pm/elixir/macro-anti-patterns.html#unnecessa...

The trouble is in finding this information, as it can be a lot to absorb. If anyone finds we should link to them from other places, pull requests are welcome. In general, PRs to improve docs are always gladly received, be in Elixir, Ecto, or elsewhere!

-------

OTHERS

> Anecdotally, when Elixir started off there was some bad blood between them and the Erlang community, which is the origin of this schism

No bad blood, really. I asked the Rebar team (not the current Rebar3 team) if they would accept PRs to also compile Elixir, they said no (which is understandable) and then we move forward with Mix (which was a contribution from a Clojure developer inspired by Lein). The projects drifted apart but we often share whatever we can in other places (such as https://github.com/hexpm/hex_core).

> In fact the Elixir compiler almost never gives you an outright error, basically it only fails if a file can’t be parsed. This feels spooky as hell… but its warnings are basically always correct and seldom miss anything

Yes! Our goal is to avoid halting compilation as much as possible and instead rely on precise warnings. It is easier to debug a program that compiles (and then raises) than one that does not compile at all.

If you ever get to what is bothering you on unit tests, I'd love to hear (feel free to reach out).

Re: ElixirNitpicks

#52

Post by conradfr »

ch4s3 wrote: post #11

> I do wish migrations were just generated from schemas though, a la Django. This is a weird one to me. I really dislike the magical way Django does this and I'm glad Ecto doesn't. It also allows you to have separate Ecto structs representing different parts of a table in scenarios where that is desirable.

I mean that could just be a mix task that you would be free to use or not.

Currently it's true (for me) that writing the migration is copy pasting the fields from the ecto schemas and modifying them because the dsl is "almost the same but not totally".

Re: ElixirNitpicks

#53

Post by Terr_ »

ollysb wrote: post #26
pg_bot wrote: post #15

The following code could be written much better by using the cond operator. with {:is_email, true} {:error, :bad_request} {:is_available, false} -> {:error, :conflict} end cond do !email_address?(email) -> {:error, :bad_request} !EmailAddresses.available?(email) -> {:error, :conflict} true -> {:ok, email} end This gets rid of unnecessary duplication, and I think is easier to understand.

Keathley did a good job discussing this in https://keathley.io/blog/good-and-bad-elixir.html#:~:text=Av... . The preferred style is to specify the errors in separate functions e.g. def main do with {:ok, response} where call_service, decode and store_in_db return the specific errors like {:error, :bad_request}, {:error, :conflict}.

I've seen that recommendation often but I still dislike it, since it falls apart whenever you need to do the same logic on multiple pieces of data. It only makes sense if every single with-clause is doing something hugely different.

A simple example:

    with {:ok, cleaned_first}   final}
    else
      {:error, :illegal_character} ->
        {:error, :illegal_first_name_or_middle_name_or_last_name}
      other ->
        other
    end
The above sucks because it's not clear which data was bad or which step failed.

So let's talk about the alternatives...

__________________

First, the "Lots Of Methods" approach, where you just add as many different private methods as you need, where each one works almost the same except for a distinctly different error:

    {:ok, cleaned_first}
I feel that sucks because it's tedious, error-prone boilerplate, even if they're just wrappers around a sanitize/1 that does the real work. Also any new error-atoms being introduced are scattered down the file rather than near where you might want to match/test them.

__________________

Second, the "Augment One Method With Causal Data" version:

    {:ok, cleaned_first}   {:error, :illegal_first_name}
This is a marginal improvement, but we're still contaminating methods like "sanitize" with junk they don't actually need to know in order to do so their job, passing a piece of opaque data down and up the stack unnecessarily.

__________________

Third, what if we carefully isolate the concerns/complexity to the with-statement... Hey! We're right back to where we started!

The "Augment The With Clauses" approach, which I argue is least-bad:

    {_phase, {:ok, cleaned_first}}    {:error, :illegal_first_name}
    {:phase_middle, {:error, :illegal_character}} -> {:error, :illegal_middle_name}
    {:phase_last,   {:error, :illegal_character}} -> {:error, :illegal_last_name}

Re: ElixirNitpicks

#54

Post by turboponyy »

> Rust-style returned tuples of {:ok, val} or {:error, e}.

Tuples fall under product types. Rust's APIs return errors as sum types. A more apt comparison would be with Go, where errors are actually returned in tuples.

Edit: nevermind, I see what the author meant now!

Re: ElixirNitpicks

#55

Post by ollysb »

karmakaze wrote: post #31
ollysb wrote: post #26

Earlier quoted context omitted.

Keathley did a good job discussing this in https://keathley.io/blog/good-and-bad-elixir.html#:~:text=Av... . The preferred style is to specify the errors in separate functions e.g. def main do with {:ok, response} where call_service, decode and store_in_db return the specific errors like {:error, :bad_request}, {:error, :conflict}.

Isn't there a way of piping the ok results through? e.g. data |> call_service |> an_ok_unwrapper(decode) |> an_ok_unwrapper(store_in_db) Or is `with` the way of doing this for ok/error results?

Doing it with `with` is definitely the community standard. There are attempts like https://github.com/vic/ok_jose to make it more like "do" notation.

Re: ElixirNitpicks

#56

Post by karmajunkie »

xanthor wrote: post #38
karmajunkie wrote: post #33

Earlier quoted context omitted.

`with` is more widely used, i think, but there's also `Kernel.then/2`: data |> call_service |> then(fn {:ok, decode} -> decode_file(decode) end) |> then(fn {:ok, file} -> store_file(file) end) your solution also works if you define the function headers with a pattern match against the tuple, but then you have this extra function hanging around. Feels like a style thing more than anything else.

This approach is not equivalent since it uses a strict match in the function head inside `then`. It will raise a `FunctionClauseError` if a value not matching `{:ok, _}` is passed in.

I mean, yes, you're right that the failure mode of the `then/2` approach works differently than `with`. I think most semi-experienced elixir devs would recognize that the function used with `then/2` needs to have a pattern that matches expected returns—as does `with/else` if you want to be able to continue on the happy path of your program.

For those that don't realize that, caveat lector.

Re: ElixirNitpicks

#57

Post by mikhailfranco »

> Elixir has no early returns

It's true there are few in the standard library, but look at 'Enum.reduce_while/3'.

Use of throw/catch for prompt return is possible, but is not considered good style, except perhaps if the recursion is very deep, or through an intermediate library layer that does not support prompt returns.

However, it's trivial to write your own prompt returns. It is a standard idiom (esp. in Erlang) to write a function as:

1. Public head, argument tests in guards, immediately call internal private implementation function, with additional empty accumulator argument.

2. Private function (clause 1), continuation case, does the work, then decides whether to: increment accumulator (often O1 prepending head to linked list) and recurse; or prompt return of some result, like found search, or error when some condition exceeded.

3. Private function (clause 2), termination case, empty arguments, reached the end of inputs, just return accumulator, often Enum.reverse of a list.

For example, specialized version of Enum.reduce_while over a list might be implemented as:

  def foo(xs) when is_list(xs), do: do_foo(xs, [])
  
  defp do_foo([x|xs], out) do
    case bar(x) do
      {:ok, val} -> do_foo(xs, [val|out])
      :error -> :error
    end
  end

  defp do_foo([], out), do: {:ok, Enum.reverse(out)}
I think when you grok this simple pattern, you will never use, Enum map, filter, reduce, reduce_while, flat_map, .... ever again, because it can emulate all of them.

Re: ElixirNitpicks

#58

Post by Miner49er »

OkayPhysicist wrote: post #49
Miner49er wrote: post #48

Earlier quoted context omitted.

This is a great example of why the pin-operator is bad, IMO. Rebinding isn't worth this complication.

On the contrary. I'd like the pin operator regardless of whether rebinding is allowed or not. def func() do s = g() ...several lines of code l = t() case l do {s, q} -> #blah {q,q} -> #blah {p, r} -> #blah end end Without the "^s", you need context to determine "s"'s behavior. If s is currently unbound, then it'd be assignment. If it's unbound, it'd be pattern matching. It's a rare enough operation that it's nice to…

I do agree with the pin-operator, after more thought. I actually think it's good, it's just the rebinding that I don't like.

I understand the argument for rebinding, as an Erlang developer I have to deal with not having it all the time. I guess I don't really see it as that big of a deal to have to use more variable names. The way I see it, if you're transforming something, it's fine for it to have a different name. I haven't used elixir, but I would guess pipelining covers 95% of the time that rebinding would be wanted, and the other 5% of the time I think I would prefer not to have it, but it's really just nitpicking, it's not too big of a deal.

Re: ElixirNitpicks

#59

Post by zachdaniel »

bmelton wrote: post #27
ch4s3 wrote: post #11

> I do wish migrations were just generated from schemas though, a la Django. This is a weird one to me. I really dislike the magical way Django does this and I'm glad Ecto doesn't. It also allows you to have separate Ecto structs representing different parts of a table in scenarios where that is desirable.

Ash Framework gives you the ability to define the schema in code, via mix ash_postgres.generate_migrations --name name_of_my_migration but of course now you're using a framework (albeit a really good one) that is perhaps a bit less mature than Elixir

I think its also important for people to realize that

1. its opt-in, if you don't like it don't use it :)

2. we don't do magic migrations. We generate a regular ecto migration from your resource changes. So you can use it as a starting point and hand edit it (and often you should or even must as we can't do things like generating data migrations).

3. you don't have to map your resources one-to-one with tables. We support multiple resources being sourced from the same table, and turning on and off migration generation per resource. It's very common to have an Ash resource backed by a postgres view, or two ash resources backed by the same table. The migration generator merges resource definitions together to create "the strictest table definition that supports both resources". You can also remap field names. So at the end of the day, you can have your cake and eat it too on that front.

Re: ElixirNitpicks

#60

Post by xanthor »

karmajunkie wrote: post #56
xanthor wrote: post #38

Earlier quoted context omitted.

This approach is not equivalent since it uses a strict match in the function head inside `then`. It will raise a `FunctionClauseError` if a value not matching `{:ok, _}` is passed in.

I mean, yes, you're right that the failure mode of the `then/2` approach works differently than `with`. I think most semi-experienced elixir devs would recognize that the function used with `then/2` needs to have a pattern that matches expected returns—as does `with/else` if you want to be able to continue on the happy path of your program. For those that don't realize that, caveat lector.

If that's the behavior desired, experienced Elixir devs would use the match operator since it's more conventional, requires fewer characters, and eliminates multiple unnecessary function calls. The thread was about monadic transformation operations similar to `and_then` on Result and Option in Rust or Promise#then in JavaScript.
Post reply on HN