Order class methods above instance methods

[jessieay]

• Didn't see this in guides anywhere, but it's something we all do

[gylaz]

I prefer grouping methods that share logical scope together, over alphabetically. For example:

class Car
  def clean_seats
    ...
  end

  def remove_seats
    ...
  end

  def install_windshield
    ...
  end

  def break_windshield
     ...
  end
end

[gabebw]

I like class methods at the top.

Within the class/instance groupings, I (like Greg) order by relationship instead of alphabetically.

[mcmire]

👍 to putting class methods on top.

👎 to ordering methods alphabetically, I think this goes too far. I also find it more useful to order by relationship instead.

[joshuaclayton]

I also prefer class methods on top and methods grouped by relationship instead of alphabetically.

[dgalarza]

👍 for class methods at the top, 👎 for ordering methods alphabetically. I prefer to group by relationship as well.

[teoljungberg]

👍 for class methods on top, I care less about the order of the instance methods

[jessieay]

wow, this is so interesting! In code reviews I've been involved with, the feedbacl has always been to alpha order methods (aka - class methods in alpha order first, then instance methods in alpha order)

Question for @teoljungberg @dgalarza @joshuaclayton @mcmire @gabebw @gylaz

• if you are grouping methods by relationship, how do you determine the order of groups?

[derekprior]

Alphabetical order seems rather arbitrary. I tend not to worry too much about public instance method ordering other than initialize going first. If method a relies on method b then b should come after a in the source. I order private methods by the order in which they appear in the source when reading top to bottom.

I don't see much to be gained by organizing alphabetically.

[gabebw]

If I have 3 methods that all deal with the same thing (where "same thing" is kind of fuzzy), I'll put them next to each other. I also usually put dependent methods below the methods that use them (maybe this is from Uncle Bob Martin?). i.e. if method A calls method B, I put method A above method B, so that I can read the file top-to-bottom and have more-general methods at the top, and if I want to dig deeper, I can (but don't have to) read farther down. Example:

class Cookie
  # This is a general method that calls other methods
  def make
    buy_ingredients
    mix
    bake
  end

  # Here are some more-specific methods that I can dig into if I want.
  # Notice that these methods are in the order they were called - I don't think
  # we should have a rule about that, but it's nice to have.
  def buy_ingredients
  end

  def mix
  end

  def bake
  end
end

[gabebw]

How about replacing "zebra" with "avocado"?

[gylaz]

Then you'd have to re-order the elements!

[gabebw]

But Zebra isn't a fruit.

[nickrivadeneira]

@gabebw http://en.wikipedia.org/wiki/Green_Zebra

[gabebw]

How long did it take you to find that?

[nickrivadeneira]

Well, I started off with primary colors, so a bit longer than if I had taken the more obvious route of searching for secondary colored zebras...

[ghost]

💯

[jferris]

We have guidelines about ordering methods:

• Order methods so that caller methods are earlier in the file than the methods they call.
• Order methods so that methods are as close as possible to other methods they call.

[mcmire]

So here's a real-world example: https://github.com/thoughtbot/shoulda-matchers/blob/master/lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb. There are methods here for building/creating records, determining default values, dealing with scope attributes, and then more generic methods at the bottom of the file.

Granted, this class is in need of refactoring. If there are multiple kinds of methods in a single file, i.e. multiple groups of methods, then your class is probably violating SRP, and it's clear that ValidatesUniquenessOfMatcher has multiple responsibilities. It definitely does get more difficult to organize a class once you reach a certain number of methods. If I had an 100-method class, well sure, then alphabetizing makes sense, because who wants to keep track of what is called where. But assuming you're also trying to keep the size of your class small and focused, then in theory, I think the rules that Joe cited should be sufficient.

[teoljungberg]

I think that I try to structure them by how they are used, so if method
b is called in method a I would place a over b and so on

Which is basically what @derekprior said.

On 03/09, Jessie A. Young wrote:

wow, this is so interesting! In code reviews I've been involved with, the instructions has always been to alpha order methods by group (aka - class methods in alpha order first, then instance methods in alpha order)

Question for @teoljungberg @dgalarza @joshuaclayton @mcmire @gabebw @gylaz

• if you are grouping methods by relationship, how do you determine the order of groups?

---

Reply to this email directly or view it on GitHub:
#320 (comment)

[jyurek]

I'm almost categorically against ordering arbitrarily-ordered things (especially methods) alphabetically: It doesn't aid understanding while scanning the file, it only kinda helps to place a new method (though that shouldn't be an arduous task, since you should have small classes), and it's a PITA because you can't just :!sort.

@mcmire: In that case, I would start with the logical entry points of the API at the top and put their dependents below it, as Joe said. If the public methods do not depend on each other, I would put them in the order I would want someone to read about them in an API doc. As you say, the class is in need of refactoring, so instead of implementing a guideline for the order of methods it should be refactored.

[gfontenot]

I had to order methods alphabetically for the entire year that I was at LevelUp and it was consistently a gigantic waste of time that only served to frustrate me during code review. I'm also less concerned with grouping class methods at the top. Put things in a logical order without being dogmatic about it and everything will be fine.

[jferris]

@jessieay any interest in updating this?

[jessieay]

@jferris yeah I can do it on Friday when I am not on client work

[jessieay]

Seems like there is general consensus on "class methods above instance methods" for Rails, so kept that in. Removed stuff about alpha order. Thanks for weighing in, all. @jferris ready to merge?

[derekprior]

Just noticed this is in the Rails style guide but there's nothing Railsy
about it. Move to Ruby?

On Fri, Mar 13, 2015 at 2:55 PM, Jessie A. Young notifications@github.com
wrote:

Seems like there is general consensus on "class methods above instance
methods" for Rails, so kept that in. Removed stuff about alpha order.
Thanks for weighing in, all. @jferris https://github.com/jferris ready
to merge?

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

[jessieay]

@derekprior good 📞 - updated

[gabebw]

I approve of this change.

[jessieay]

anyone else?

[nickrivadeneira]

🚀

[jessieay]

Merged! thank you for your feedback, everyone.