How we should break the long arguments line with no parentheses? about guides HOT 19 CLOSED

I'd rather see:

attr_accessor :email, :name, :github_username, :password, :stripe_customer_id
attr_accessor :stripe_subscription_id, :stripe_token, :organization, :address1
attr_accessor :address2, :city, :state, :zip_code, :country

from guides.

I've most often seen the no-parenthesis example with "macro" methods, like attr_accessor or delegates, which are actually class methods. Maybe the two styles help people remember whether they are operating at the class vs instance level?

Would you also want to see:

redirect_to \
  edit_user_url(@user),
  notice: t("flashes.users.create.success")

from guides.

Another nice side effect of 2nd example is you can use vim command :sort to sort them alpha.

attr_accessor(
  :address1,
  :address2,
  :city,
  :country,
  :email,
  :github_username,
  :name,
  :organization,
  :password,
  :state,
  :stripe_customer_id,
  :stripe_subscription_id,
  :stripe_token,
  :zip_code,
)

from guides.

I would be fine with:

attr_accessor \
  :address1,
  :address2,
  :city,
  :country,
  :email,
  :github_username,
  :name,
  :organization,
  :password,
  :state,
  :stripe_customer_id,
  :stripe_subscription_id,
  :stripe_token,
  :zip_code

(also easy so sort, as Harlow mentions)

from guides.

In factories the second version looks better: it's not only an enumeration, they are options, symbols, strings, hashes, etc; a line per argument seems more clear.

In attr_accessor Prem's first version looks better IMO.

from guides.

I like the backslash aesthetic and sorting,

On Nov 13, 2014, at 8:03 AM, Joe Ferris [email protected] wrote:

I would be fine with:

attr_accessor
:address1,
:address2,
:city,
:country,
:email,
:github_username,
:name,
:organization,
:password,
:state,
:stripe_customer_id,
:stripe_subscription_id,
:stripe_token,
:zip_code
(also easy so sort, as Harlow mentions)

—
Reply to this email directly or view it on GitHub.

from guides.

👍 to @jferris and the backslash.

Related: I've typically done long validators like this (which Hound complains about).

validates :phone_number,
  presence: true,
  uniqueness: true,
  length: 10,
  numericality: { only_integer: true }

With this style guide, would this be the way to go?

validates \
  :phone_number,
  presence: true,
  uniqueness: true,
  length: 10,
  numericality: { only_integer: true }

For validators, I kinda like the aesthetic of having the attribute name on the same line as validates, and then a list of the validations aligned underneath it. Grouping the attribute name in with the list of hash arguments looks a bit weird to me, besides the fact that you definitely don't want to include it in a :sort.

from guides.

Another benefit of:

attr_accessor(
  :address1,
  :address2,
  :city,
)

is less diff noise when another argument is added.

from guides.

@gylaz huge thumbs up on the trailing comma!

To @mike-burns's point, what I've done in the past is break attr_* calls up on multiple lines, sorted alphabetically, grouped logically:

class PropertySerializer < ActiveModel::Serializer
  # ... 
  attributes :id, :name, :chain_slug, :description, :pegasus_property_id
  attributes :left_preview_image_url, :right_preview_image_url, :thumbnail_url, :brand_tier_logo_url
  attributes :largest_meeting_room_capacity, :meeting_rooms, :total_rooms, :total_meeting_room_area, :total_meeting_room_area_units
  attributes :street_one, :state, :city, :zip_code, :country, :phone_number, :latitude, :longitude

(Newlines removed to demonstrate groupings)

The downsides to this is that you have to identify groups, and if you're adding an item, determining what group it falls into, or if it belongs in a new group (and what if other options previously defined fall into this new group!), which alpha sorting nullifies completely.

from guides.

We don't have a requirement for alphabetically sorting such things.

from guides.

I prefer the escaped:

attr_accessor \
  :address1,
  :address2,
  :city,
  :country,
  :email,
  :github_username,
  :name,
  :organization,
  :password,
  :state,
  :stripe_customer_id,
  :stripe_subscription_id,
  :stripe_token,
  :zip_code

I like sorting.

I have the same issue as @Graysonwright with validators and, most frequently, factories. For some reason, this looks ugly to me:

method(
  :symbol,
  and: "some",
  hash: "args"
)

# assume 1..n initial arguments and a bigger options hash

But I have no good solution. I would almost prefer:

method(:symbol, {
  and: "some",
  hash: "args"
})

But Hound balks at the redundant curlies.

from guides.

We don't have a requirement for alphabetically sorting such things.

It's not a requirement, but it's a benefit. A decent number of people prefer to sort lists like these when no other meaningful order exists, so being able to do so quickly is nice.

from guides.

Hound balks at the redundant curlies.

Hound will do whatever we want it to!

This looks ugly to me

Our current guidelines are mostly subjective, aesthetic preferences and are always open to change. I think it's just good to keep in mind that, since aesthetic preferences are personal and vary, any guideline will create code that looks nicer to some folks and uglier to others. With that in mind, I'd generally prefer less churn.

That being said, I think it would be reasonably harmless to have an alternative (like the \ version with no parenthesis), because it wouldn't invalidate any existing code. This may be one of the cases where the option for a little variety outweighs the need for consistency.

from guides.

But I have no good solution. I would almost prefer:

I think this strategy breaks down when:

some_longish_method_name(:some_argument_that_causes_this_line_to_be_over_80_chars, {
  one: 1,
  two: 2,
})

The above example, is why I think there's a benefit to each argument on its own line guieline.

from guides.

method :arg1,
  hash_opt1: val,
  hash_opt2: val

I see no problems with this. It keeps things mostly in the same state (2-space indenting), it prevents the (frankly) ugly backslash, and it has the benefit of logically grouping the first arg with the method, which is often what makes sense. Yes, there are times when it breaks down, but we should not optimize for those situations. We want to have a guideline for the 80%, not the 20%.

And on the subject of logically grouping things, I strongly dislike requiring an alphabetized list.

from guides.

@jyurek Does that rule apply in the following situation too?

method :arg1,
  :arg2,
  :arg3

Or, in this case, do we put each argument on its own line? Doesn't seem consistent to me.

from guides.

I'd be ok with that, but I think in the situation where things don't fit on one line, we might want to think about some other way anyway.

from guides.

But Hound balks at the redundant curlies.

But Hound barks at the redundant curlies.

from guides.

This conversation has gotten stale, so I'm going to close this. Feel free to open a pull request if you'd like to update the style guide.

from guides.