IndentationWidth opinion for Ruby 2.1 is questionable

[chendrix]

Because of the change in Ruby 2.1 that causes method definitions to return a symbol of their method name, we can now do things like

private def foo
          # ...
        end

cached def bar
         # ...
       end

def self.cached(method_sym)
  # ...
end

Now this may just be my opinion, but things like private def foo is much preferable than

private
# EVERYTHING BELOW HERE IS MAGICALLY PRIVATE
def foo
  # ...
end

and you couldn't even do the ingenious cached def bar before.

So this brings up the question of preferences between two styles:

private def foo
          # ...
        end

# vs.

private def foo
  # ...
end

I personally much prefer the former as it specifically highlights the distinct nature of this kind of method declaration.

If you do it this way, RuboCop complains about the IndentationWidth being larger than 2, about the EndAlignment, and about EmptyLinesAroundAccessModifier, which I question.

[mikegee]

I prefer your second option:

private def foo
  # ...
end

I'd like to throw in a couple more options, just for completeness (not because I like them):

private \
def foo
  # ...
end

private(
  def foo
    # ...
  end
)

[jonas054]

@chendrix We could definitely add a configuration option to support the style you prefer. Maybe it would be better to extract the def-end alignment checks to a new cop since the configuration option would only apply to that particular case.

@mikegee I think the extra examples you supplied will be covered too (maybe not the last one) if we do something like

DefEndAlignment:
  AlignWith: start_of_line
  SupportedStyles:
    - start_of_line
    - def

[jdickey]

I agree with @mikegee. Weigh the benefit o having the access modifier be so screamingly obvious against the sacrifice of a good chunk of the maximum line length. Looking at the code I work with, I'd rather have the space.

Besides,

1. Nothing beyond bad style prevents you from doing

private

def foo
  # ...
end

public

# ...

2. alternatively, and in accordance with the usage I've seen, written and tend to prefer:

class Widget

# ...

protected

def foo
# ...
end

private

# ...

end # class Widget

I've been using that system since the UCSD p-System days and IWFM. Thoughts?

[chendrix]

Weigh the benefit o having the access modifier be so screamingly obvious against the sacrifice of a good chunk of the maximum line length.

@jdickey I personally always either disable or significantly lengthen the LineLength option anyways

I've always always hated the traditional way of access modifiers as seen in your 2). Design principles (and in particular Gestalt) state that "objects or shapes that are close to one another appear to form groups", the principle of Proximity. The opposite to this is visual Rhythm.

class Widget

# ...

protected

def foo
# ...
end

private

# ...

end # class Widget

shows significant Rhythm, as scanning with the eye shows all keywords are left aligned and that every single "chunk" (def, private, protected) is separated from the one above and below it by a single space. This means that there is no Proximity and that your protected and private sections do not get visually associated into groups.

Compare that to

class Widget

# ...

##################################
protected

def foo
# ...
end

##################################
private

# ...

end # class Widget

which breaks the visual Rhythm and promotes Proximity groupings. It's much harder to accidentally miss the access modifiers in this version than in the "best practices" one.

So instead of trying to make proximity based groupings of access control which likely can only be done with clunky comments or some other form of spacing-scheme, we can pull a style from Java-land and instead specifically tag our private and protected method

class Widget

# ...

protected def foo
# ...
end

private def bar
# ...
end

private def baz
# ...
end

end # class Widget

Then at this point it's a matter of preference between the various "tagged access" forms that myself and @mikegee stated.

[jdickey]

@chendrix: I'm actually OK with your last example, where the access modifier is simply part of the method signature (thus implying that it should be supplied for every method?). I was just voicing my disbelief that anyone would actually use your original example's indentation style as a horrendous waste.

But if we used the style you just proposed, then a) we'd need a cop to remind us when we forgot an access specifier, and b) we'd take even more flak from those who think Ruby is "starting to look too much like Java".

Finally, I wonder how much of this is really a way of avoiding addressing a code smell; i you've got more than a reasonably-sized source file, you may well have an SRP violation such as Feature Envy.I periodically go through my then-current project's sources, looking hard at any that are more than 2 sigma over the mean, or 120 lines, whichever is less. As often as not, I find that I've conflated multiple concepts unintentionally and, with a little thought, can come up with better code.

[chendrix]

1. I certainly think every private & protected method should be tagged
2. it's only a "waste" under your specific constraints
3. I don't think we specifically need a cop to enforce my style. I'm not asking for it to be adopted or even checked, I'd just like to be able to program in my style without needing to disable IndentationWidth, EndAlignment, and EmptyLinesAroundAccessModifier cops.
4. Who gives a crap what the "Ruby is starting to look too much like Java" people think? Are we often in the business of preventing experimentation and looking for better practices just because someone's upset that NEW_HOT_LANGUAGE is starting to look like OLD_STODGY_LANGUAGE
5. I can assure you it's absolutely not a way of avoiding addressing a code smell. I see the benefits of this style in all of my classes, including my latest project where the largest file is 63 lines long.

[ghost]

I see the benefits of this style in all of my classes, including my latest project
where the largest file is 63 lines long.

And others like me won't see any benefit, but sure enough would
object to private tags being put before a method definition.

Why not this here instead?

def foo
end; private :foo

I prefer this style, or:

private

def foo
end

Is also ok.

But:

private def uargh

is awful.

I however concur that RuboCop should be easily able to allow you to
define your own constraints.

BTW chendrix on your comment at
May 17, 2014

you even used another style than you originally
proposed. :-)

[jdickey]

@shevegen Agreed on all counts. @chendrix, when in your initial comment you said

Now this may just be my opinion, but things like private def foo is much preferable than ...

you were exactly right; and I think that most of the folks pushing back against your new style probably don't see your cached bit as "ingenious" beyond the sheer "let's see how far we can push the language before it breaks" nihilism of it all.

I hereby move that the issue be closed without action.

[chendrix]

So will Rubocop move to not implement all things its core developers deem
heretical and against their personal style opinion? Or did I have the wrong
impression that Rubocop was supposed to be moldable by clients to fit their
own style guides.
On Jun 3, 2014 11:21 PM, "Jeff Dickey" notifications@github.com wrote:

@shevegen https://github.com/shevegen Agreed on all counts. @chendrix
https://github.com/chendrix, when in your initial comment you said

| Now this may just be my opinion, but things like private def foo is much
preferable than ...

...and I think that most of the folks pushing back against your new style
probably don't see your cached bit as "ingenious" beyond the sheer "let's
see how far we can push the language before it breaks" nihilism of it all.

I hereby move that the issue be closed without action.

—
Reply to this email directly or view it on GitHub
#1092 (comment).

[chendrix]

Also @shevegen

Why not this here instead?

def foo
end; private :foo

I prefer this style, or:

I have to believe you're being contrarian for the sake of being contrarian if you seriously consider that a viable, aesthetic, and meaningful alternative. It's another one of the many styles where you lose scannability of your access modifiers.

[jonas054]

@chendrix I'd be happy to implement something similar to what I proposed earlier in this discussion. I've been busy with bug fixes for a while and am now on vacation, and that's the main reason why it hasn't been done already.

@mikegee @jdickey @shevegen You've all argued why you prefer the currently enforced style. This is interesting but not crucial, since we'll keep that style as the default. Only if @chendrix's style didn't hold up logically would it be proper to close the issue without action. I've seen nothing that suggests it is so; it's just a matter of preference.