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

support for new private syntax #730

Closed
fgarcia opened this issue Jan 11, 2014 · 10 comments
Closed

support for new private syntax #730

fgarcia opened this issue Jan 11, 2014 · 10 comments

Comments

@fgarcia
Copy link

fgarcia commented Jan 11, 2014

Since Ruby 2.1 this new syntax is valid to define private methos

private def map(attribute, to:)
  @rules[attribute] = to
end

However, currently the cop EndAlignment complains in this case

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 14, 2014

That's not actually a new syntax. It's just a side effect from the fact that in 2.1 def returns the method name. I'm not sure what's the best way to approach this. I'm definitely not particularly fond of this code layout as it seems cluttered to me and it would require a private for each method.

@jonas054
Copy link
Collaborator

I think I like the new way of using private that 2.1 adds.

In this book Clean Code, Robert C. Martin writes that "Concepts that are closely related should be kept vertically close to each other". He also talks about the "Newspaper metaphor", which means that the important stuff should come first in a class, and smaller and smaller details further down.

If you follow this style, you write a public method that calls some private methods. These private methods come directly after the public method. Then comes the next public method. If a private method is called from several methods, it's placed after the last call. All calls are downwards in the file.

That's how I like to write code and in languages like Java, C++, and Python it's natural to do so. Ruby is biassed towards having all the public methods first and all the private methods last. You don't have to, but that's what we do because it seems easier (easier than using private :my_method1, :my_method2).

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 15, 2014

Personally, I'm fond of grouping the public methods together, but I understand your point. In Java I'd occasionally place related private methods right after the public method that uses them. That said, I don't understand about the point about C++, since when I was a C++ programmer about a decade ago methods had to be grouped there more or less the same as they are in Ruby (in public, private, protected sections).

Anyways, I guess we can allow this indentation style if it's simple enough to do so. I'm also worried about the editor support for this indent style. I guess now most would indent the code like this anyways:

private def map(attribute, to:)
          @rules[attribute] = to
        end

As the def in the argument to private some might argue that this indentation is more "correct".

@jonas054
Copy link
Collaborator

About C++ (a bit off-topic for this project). It's only in the class definition (where you usually just declare the methods, normally put in a header file) that you have sections (public/protected/private). The method implementations can come in any order.

About the indentation in editors. Good point!

@fgarcia
Copy link
Author

fgarcia commented Jan 15, 2014

Personally I do love being able to refactor out small private functions from my code and leave them next to the related code. Also I think it should look like:

        private def map(attribute, to:)
          @rules[attribute] = to
        end

Maybe it is just a matter of taste, but leaving so much indentation space, will eat a lot of screen. Wouldn't it be better just one single indent level, like a normal function? Visually it would match with other functions on the same namespace. Even using a block of private/protected does not use an extra indent to keep its functions at the same level as the others

According the Ruby style guide:

https://github.com/bbatsov/ruby-style-guide

Indent the public, protected, and private methods as much the method definitions they apply to.

so why not do the same for their content?

As far as editor support, it is no longer an issue for Vim users, or at least... a closed issue: vim-ruby/vim-ruby#190

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 16, 2014

@jonas054 Emacs support for this is brewing :-)

@jonas054
Copy link
Collaborator

That makes me very happy.

@jonas054
Copy link
Collaborator

Getting back to the issue at hand... I'll update EndAlignment to enforce the style in @fgarcia's example when private, protected, public, or module_function is on the same line as def. Not configurable. OK?

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 16, 2014

Yes.

On Thursday, January 16, 2014, Jonas Arvidsson notifications@github.com
wrote:

Getting back to the issue at hand... I'll update EndAlignment to enforce
the style in @fgarcia https://github.com/fgarcia's example when private,
protected, public, or module_function is on the same line as def. Not
configurable. OK?


Reply to this email directly or view it on GitHubhttps://github.com//issues/730#issuecomment-32545573
.

Best Regards,
Bozhidar Batsov

http://www.batsov.com

bbatsov added a commit that referenced this issue Jan 17, 2014
[Fix #730] Enforce end aligned with "private def", etc, in Ruby 2.1.
@bbatsov
Copy link
Collaborator

bbatsov commented Jan 17, 2014

Btw, Emacs support for this is now merged upstream and will be part of the upcoming 24.4 release.

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

3 participants