Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More guidance for indenting, aligning in case statements. #157

Closed
raarts opened this issue Oct 17, 2017 · 3 comments
Closed

More guidance for indenting, aligning in case statements. #157

raarts opened this issue Oct 17, 2017 · 3 comments

Comments

@raarts
Copy link

raarts commented Oct 17, 2017

(I'm taking case statements as an example, but it can be applied elsewhere. Feel free to adjust the title of this issue.)

I feel the style guide can be more explicit about the following:

    case env.method do
      :get -> %{env | status: 400, body: Poison.encode!(%{id: "Sdds"})}
      _ -> %{env | status: 405, body: Poison.encode!(%{status: "error"})}
    end

Now, I find this ugly, so I ended up with this, which I find much easier to read.

    case env.method do
      :get -> %{env | status: 400, body: Poison.encode!(%{id: "Sdds"})}
      _    -> %{env | status: 405, body: Poison.encode!(%{status: "error"})}
    end

And yes, you might need to change adjacent lines when you change the first clause.

It gets more problematic when one of the case clauses is textually long:

   case env.method do
     :exceptionally_long_atom -> %{env | status: 400, body: Poison.encode!(%{id: "Sdds"})}
     _ -> %{env | status: 405, body: Poison.encode!(%{status: "error"})}
   end

or, when aligning:

   case env.method do
     :exceptionally_long_atom -> %{env | status: 400, body: Poison.encode!(%{id: "Sdds"})}
     _                        -> %{env | status: 405, body: Poison.encode!(%{status: "error"})}
   end

The style guide seems to favour (from the examples):

   case env.method do
     :exceptionally_long_atom ->
       %{env | status: 400, body: Poison.encode!(%{id: "Sdds"})}
     _ 
       -> %{env | status: 405, body: Poison.encode!(%{status: "error"})}
   end

Which, to me, is really ugly and not easy to read.

Not really knowing where I'm going with this, just something I bumped into when trying to make my code match the style guide.

@christopheradams
Copy link
Owner

Lots to consider. As an aside, I'm guessing you meant to format the last example like this:

   case env.method do
     :exceptionally_long_atom ->
       %{env | status: 400, body: Poison.encode!(%{id: "Sdds"})}
     _ ->
       %{env | status: 405, body: Poison.encode!(%{status: "error"})}
   end

@raarts
Copy link
Author

raarts commented Oct 19, 2017

Yes, you're right. Sorry about that. Still think that's ugly though, especially with more clauses.

@christopheradams
Copy link
Owner

The preferred styles for case statements based on the Elixir code formatter are:

case env.method do
  :get -> %{env | status: 400, body: Poison.encode!(%{id: "Sdds"})}
  _ -> %{env | status: 405, body: Poison.encode!(%{status: "error"})}
end

and:

case env.method do
  :get ->
    %{env | status: 400, body: Poison.encode!(%{id: "Sdds"})}

  _ ->
    %{env | status: 405, body: Poison.encode!(%{status: "error"})}
end

Hopefully the latter one addresses your readability concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants