Closed EmilyMB closed 9 years ago
I'd seen these rules before, but it's funny that I hadn't thought about the issue of link_to requiring multiple parameters as breaking the rules. Personally, I have had a hard time keeping methods to 5 lines (and keeping spec files to 100).
Related listen if anyone is interested, here's a podcast that Sean Griffen at thoughtbot released about trying to follow these rules.
It seems like two on this list could be tricky. The 5 line method rule seems like it could be tough largely in the case of if/else's. I try to follow this one as closely as possible on a normal basis, and It's hard to argue against extracting in to private methods, but there are times that I worry that over abstraction can make code harder to follow...
The other one that I think might prove challenging is only instantiating one object in a controller. The Facade Pattern that they talk about seems fairly straightforward, but it seems like one of those things that would take focused attention to remember to do all the time. This one also seems somewhat borderline to me for the reason I mentioned above (difficult to follow abstraction).
I believe I would struggle most with the five line methods in the case of if/else's. My first reaction to initiating a single object in the controller was similar but I like how they worked around it using the facade pattern.
I like the point that private methods need to have extremely descriptive names, but I don't think this is immediately embraced by all devs. I've been told to make methods descriptive, and when I've made longer more descriptive methods names, I've been told the names were too long. There's more subjectivity besides this--just the fact that any rules can be bent if your pair or code reviewer agrees they can. I don't think this is necessarily a bad thing, since I do believe that having more rules (similar to writing poetry) makes one more deliberate about their choices.
We had a Monday session on this a few weeks ago and it really stressed me out. There are some cases that just don't fit the rules, and I was having a hard time trying to make them which was causing some weird hacks. Now I'm starting to realize it's ok to break the rules, as long as you put through the effort and are sure it's the best way of handling the problem.
The one rule that pops out to me immediately would have to be keeping controllers down to one instance per controller action. I found this task more difficult, especially after messing with HTML modals and the like. Developing on a deadline can honestly make any of these rules a little more challenging.
I like all these rules except the 5 line methods when using if/else/elsifs. If you don't include those in the line count, it gets much more interesting
I think quite a few of these rules are easy for us to follow as junior developers working on apps that have a lifespan of ~3 weeks. It'll be curious how it will affect our style once we are all in jobs with code that has persisted for many months or even years.
I haven't run into any issues keeping classes less than 100 lines (although I have run into that problem with spec files, so I clearly need to separate those out more).
@DSynergy, why don't you like including if/else/elsif
in the Five Lines per Method rule? I think it really helps you think about what your method should be dealing with, and keeps you from creating a ton of cases. elsif
is honestly my least favorite thing to see in code simply because (as this blog post points out) it probably could have been extracted into a private method itself.
@KristaANelson, do you have any specific examples you could share where it was hard to implement these rules?
I think the hardest rule for me to follow is only instantiating one object in the controller. They example the blog gave is still a bit cloudy to me; I think it would be helpful to see more of the Status
model. I'm also curious about Sandi Metz' and ThoughtBot's standpoint on creating class methods. I recently read a post that seemed to discourage them on the basis that they resist refactoring. Status.for(user)
reads very clearly, but is that actually good practice? Curious to what people think.
Discuss Sandi Metz's Rules for Developers here. Which of Sandi's rules do you feel like might be the hardest to follow—why?