How do you return your ok tuples?

I was wondering what Elixir developers think about this: how to treat piping into an ok_tuple? I have no idea if there was a similar question, I searched but I could found only posts on how to treat ok/error tuples cases.

I mean, between these, which ones would you prefer? I guess they’re all valid to a certain extent, just curious about different points of views :slight_smile:

  1. {
      :ok,
      conn
      |> assign(:attr_one, :value_one)
      |> assign(:attr_two, :value_two)
      |> assign(:attr_three, :value_three)
    }
    
  2. {
      :ok,
      assign(
        conn, 
        attr_one: :value_one,
        attr_two: :value_two,
        attr_three: :value_three
      )
    }
    
  3. conn
    |> assign(:attr_one, :value_one)
    |> assign(:attr_two, :value_two)
    |> assign(:attr_three, :value_three)
    |> (&{:ok, &1}).()
    
    

or there might be something different better, can’t think at any other right now.

3 Likes

When constructing literals (like tuples, but also lists and maps) I try to always have the elements of that literal just be simple variables, or other literals. That is to say, when reading the literal, you can just focus on its structure, and you don’t need to also read various function calls along the way. eg:

I favor

conn =

{:ok, conn}

And stuff like:

comments =  
posts = fetch_posts(...)
users = posts |> Enum.map(& &1.author)

%{
  users: users,
  posts: posts,
  comments: comments
}

If you mix a bunch of function calls inside the literal you have to read it sort of inside out.

12 Likes

I typically agree with @benwilson512 's preference there. However since Elixir 1.12 there’s a new option that I haven’t had much chance to use, so I haven’t fully formed an opinion on it.

conn
|> assign(attr_one: :value_one, attr_two: :value_two, attr_three: :value_three)
|> then(&{:ok, &1})

I prefer the single call to assign with a keyword list, and I prefer then to the anonymous function syntax used in option 3.

5 Likes

In LiveView I usually do

{:ok, socket
      |> assign(:a, a)
      |> assign(:b, b)}
1 Like

I always prefer to use the pipe operator in cases I can express clearly that a chain of transformations is going to be applied to a particular type of data. I do that by not mixing purely data transformation pipelines with control flow. When I see functions that return an ok-result or error-reason tuple, that function tells me that it wants me to make a decision based on the return value, which might have succeeded or not. For those cases, I rather use something like with to chain operations that require decision making.

I’d prefer either: 1) with a function that does the assigns for me, so: {:ok, assign_values(conn)} or returns the ok-tuple directly like maybe_assign_values(conn). I particularly prefer to split code like that and I find it to be a little more readable.

1 Like

The winner for me: 2.

I find 3. to be the most unreadable, because you don’t even see clearly the structure of what you’re returning - namely, a tuple.

Between 1. and 2., I prefer 2. because using a pipe is unnecessary in this case, as you can do everything with one single call to assign/2.

1 Like

I am a fan of 1. Something about each line doing its own thing. If it was like 2, then I would probably make an assign_somename function if I felt they belonged together, which I would then make it like option 1

2 Likes
def ok(term), do: {:ok, term}

# …

conn
|> assign(:attr_one, :value_one)
|> assign(:attr_two, :value_two)
|> assign(:attr_three, :value_three)
|> ok()
3 Likes

I prefer readability over everything else:

conn = 
  conn
  |> assign(:attr_one, :value_one)
  |> assign(:attr_two, :value_two)
  |> assign(:attr_three, :value_three)
{:ok, conn}

I tend to keep creation of tuples, maps, etc as simple as possible. Someone else who is going to read and maintain code I have written should not have any cognitive overhead.

7 Likes

Yeah I follow benwilsons and kartheeks pattern. I think it’s more readable if you have clear do-work -> return-work separation.

Though I will construct basic types in the return if they’re small enough:

{:ok, {post, comments}} # Will do

tup = {post, comments}
{:ok, tup} # Wont do

{:ok, %Error{error?: false, ok?: true, message: "there was no error"}} # just kidding
4 Likes

The “problem” with this approach is that the formatter will make it look like this:

{:ok,
 socket
 |> assign(:a, a)
 |> assign(:b, b)}

It looks much nicer the way you presented it.

2 Likes

This :point_up: The way the formatter handles those is a bit prohibitive to building the tuple inline (without intermediate variables). Seems it could be handled nicely for two-element tuples, but I’m not sure if there is a generic way to handle this nicely.

2 Likes