Method Ordering Proposal

[mattbischoff]

Here’s a proposal on how to handle the Method Ordering conundrum that’s come up in #56 and #44. Would love to hear what everyone thinks here.

I do think it makes sense to have a consistent way to do this, and this way has worked for me, but totally open to other ideas.

Closes #44.
Closes #12.

[segiddins]

@mattbischoff I would also include where in this ordering you would put the Private methods

[bcapps]

Private method implementations become public and vice versa all the time. Maybe it's just me, but I don't see much value in separating them within the implementation file, which itself is already part of a separation between the public API and the private implementation.

[segiddins]

Right, but then where do you put the (helper) methods that are not part of any public interface, and therefore naturally have no interface by which to group?

[bcapps]

In this ordering, they're grouped by class already, so I would imagine that they'd be within the section (and pragma) of the containing class.

[segiddins]

So at the bottom of the class part, before protocols?

[bcapps]

I don't think that that public and private ordering within the pragma is so important or easy to define because of the changing nature of access control on methods.

[mattbischoff]

I agree with @bcapps, since the header declares the public methods, it’s not necessary to order them separately in the implementation.

[bcapps]

👍

[grantjbutler]

👍

[jmont]

To be honest, I would never enforce this rule. I feel like it will waste more time having people think about this and argue about this in PR reviews.

[paulbruneau]

I'm probably with JC on this. I don't disagree with this, but in actual practice, I'm not going to put a note into a pull request to reorder a method, nor am I going to spend the mental cycles to verify any particular order.

I will tend to follow this order while I'm actually writing code or making changes, though, so I don't know what position my thumb is in.

[bcapps]

I feel like it will waste more time having people think about this and argue about this in PR reviews.

It's fairly deterministic, doesn't seem like much to argue about.

To be honest, I would never enforce this rule

I do think that it's worth defining a general practice for the ordering of methods and pragmas. It allows our code to be grouped in logical and predictable ways. There are so many different ways that methods are grouped in the core app, at least, and they're constantly re-arranged depending on who is working on the file. This would probably save time for us by creating a predictable framework for code ordering and reducing the arguments around ordering.

[bcapps]

I'm probably with JC on this. I don't disagree with this, but in actual practice, I'm not going to put a note into a pull request to reorder a method, nor am I going to spend the mental cycles to verify any particular order.

This probably happens for most style guidelines. People remember some points and enforce those, but not others. The real solution to that problem is to have a good linter so no one has to think about it, and @Twigz is working on that as part of the Xcode Bots process right now.

[mattbischoff]

[jmont]

from yesterday's xkcd

[jmont]

OK rephrasing my position on this PR:

I vote 👎. For the time we would spend discussing method ordering in PRs I don't think we would get enough value to be worth it. This would also become more time consuming in objects with complex hierarchies.

[mattbischoff]

@jmont The problem I see with that argument is that right now it’s possible to spend much longer discussing method ordering because there’s no deterministic way to do it. This pull is intended to completely solve that problem and therefore decrease, not increase the time spent discussing this stuff.

It will also make it easier for people who are experienced working in one project to find things in another because ordering is consistent and the Jump Bar in Xcode clearly shows you where things are.

[jmont]

That's a good point, but I recall maybe one or two instances where someone commented on the placement of a method / organization of a file in the last year or so.

I am not opposed to method organizing / grouping. It's something that's everyone should be doing. I am against adding it to the style guide since our time is better spent doing other things. This rule will drive attention to the topic, and increase discussion about it. And method ordering is not even interesting in theory or in practice, it's just yak shaving for yak shaving's sake.

Our code quality is not going to improve because we group our methods in a certain order.

[mattbischoff]

I am not opposed to method organizing / grouping. It's something that's everyone should be doing.

Cool, we’re on exactly the same page here. The problem for me arises on a growing team when you say “everyone should be doing this”, but don’t have an agreed on recommendation on how. Many grouping systems exist and picking one and sticking to it is in my mind more important than coming up with the perfect system or worse, having no system at all.

I think this is a reasonable system that’s worked well for me and others in the past, so if everyone should be doing it I figured we might as well do it this way. Totally open to other ideas here.

Our code quality is not going to improve because we group our methods in a certain order.

There’s not much I can say on this point other than that I disagree. Superficial aspects of code are a huge indicator of overall code quality and help to avoid the broken window effect. I think that not only will this ordering scheme make people think more about where they’re putting code, but it will also improve the code itself over the long term by maintaining readability and consistency.

[cdzombak]

with system protocols proceeding custom protocols

I am not sure I agree with specifying this explicitly. Often my custom protocols feel, for lack of a better term, more "important" or more "domain-specific" than the UIKit et al. protocols, which tend to be more generic and feel more like boilerplate (obviously generalizing here).

Aside from this, I'm on board with this PR; @mattbischoff, what would you think about eliminating this one phrase? I don't think that change would diminish the value of the guideline.

[cdzombak]

reposting since I accidentally added this comment on #12 instead of here:

Closes #44.

Unless I am missing something, this PR does not address ordering of class vs. instance methods.

[mattbischoff]

Looks like I owe this some love.

[mattbischoff]

More discussion going on at #120. Will fork off that and make changes if necessary.