BackerKit / BackerKitProductFlow

This is where we keep track of how we do things.
1 stars 0 forks source link

Proposition: Remove section commanding only one new-line between methods #1

Closed johntrandall closed 7 years ago

johntrandall commented 7 years ago

Typically, one new line is appropriate.

However, -sometimes- I find it helpful to add 2-3 lines to group methods togehter and distinguish them from other methods. I find that this helps draw out the relationships between the methods and highlight parallel structures.

I also find it helpful to distinguigh the top-level-happy-path, 'perform'-type do-er methods (which are sometimes a bit procedural) from component-part contributing methods. Sometimes there is reason for these lesser, almost internatl methods to not be private, and additional space helps read the code faster.

Also, sometimes there is a logical group to methods, but not quite reason enought to extract these groups out into seperate objects yet. In that case, adding additional space to make groups helps leave a rough-edge.

srt32 commented 7 years ago

@johntrandall would you be able to find an example in our codebase (make sure it's safe to make public or make up an example) that is motivating this idea?

ryanclark2 commented 7 years ago

Notes from IRL convo with John: I think think that if a class has enough methods that we need whitespace to make sense of it, we should move things out of the class. John's argument to that is that this is a place on the continuum that's before class size reduction.

Another thing about this that I just thought of: How do we know if this is intentional or just accidental whitespace at a glance? Example:

https://github.com/BackerKit/BackerKit/blob/master/app/models/backer.rb#L262

srt32 commented 7 years ago

The intention confusion is the most compelling argument for me against this change

How do we know if this is intentional or just accidental whitespace at a glance? Example:

srt32 commented 7 years ago

I propose we close this PR.

ryanclark2 commented 7 years ago

Closed for now - I think we like 1 line as often as possible.