turingschool-examples / intermission-assignments

33 stars 218 forks source link

Read Sandi Metz's Five Rules for Developers #2

Closed stevekinney closed 9 years ago

stevekinney commented 9 years ago

Read: Sandi Metz' Rules For Developers (10 minutes)

Bonus challenge: As you implement Jet Fuel this week (#1), see if you can follow Sandi's rules to the letter.

bayendor commented 9 years ago

I try to follow all of them, but 'five lines per method' is problematic. Learning to refactor to pull out extra logic is a skill I am working on.

The most difficult is probably 'only instantiate one object in the controller', which I'm pretty sure is broken in a Rails app all the time, or I'm reading it wrong.

skuhlmann commented 9 years ago

I typically try to follow the short class and method rules as much as possible. I also don't think I've written a method outside of my rails views that took more than four arguments.

Only allowing one model instantiation per controller seems most difficult to me, but I like the idea of the facade pattern that the author explains.

VikiAnn commented 9 years ago

Can I just ditto David and Sam? (We're all Turing students, we're likely to have at least somewhat similar styles, right?) Because the one model instantiation per controller and five lines per method rules are the two I thought of as well, for the same reasons.

stevekinney commented 9 years ago

@skuhlmann, @VikiAnn, @bayendor: The only sending one object to the view is definitely the hardest for me. Granted, I cheat these days by using Rails for my API and then hitting the API from my front-end. It's a lot easier when you're not building the views on the server.

I probably should have included it in the homework but Sean Griffin and Derek Prior at thoughtbot did a podcast on trying to follow Sandi's rules. It's definitely worth a listen if you're interested in this kind of stuff.

bayendor commented 9 years ago

@stevekinney Yes, I listen to the Bike Shed podcast, and enjoyed that episode. I am currently working my way through Sandi's book POODR, and it's a tough conceptual leap to make, but I think reading it now with some development under my belt is the right time for it start sinking in.

stevekinney commented 9 years ago

@bayendor The funny thing about Sandi's book is that for such a thin book it's incredibly dense with rich material and insights.

bayendor commented 9 years ago

@stevekinney Agreed. I find myself reading it, seeing no difference in the last thing I did. They working through the code, and seeing the very subtle thing that was just changed, and then (I suppose) some learning happens...

dalexj commented 9 years ago

Assuming controllers are allowed to create objects that can accept nested attributed, I don't think I've set up more than 1 model in my controllers. Pretty sure I've kept my parameters 4 or under.

The other rules I find a lot harder to follow/ don't solve anything directly. Something like having a 5 line method will force you to fix other problems with your code, but I think there are plenty of places where 7 lines of code will read much better than trying to break it into 6 methods and having to follow them everywhere.

trayo commented 9 years ago

I feel like it's easy to follow rule three about not passing in more than four parameters if you're also following the rule about methods being no longer than five lines.

I like the one about only passing one instance to a view. It should keep logic out of the view and in the controllers and models.

Is the rule about not having classes over 100 lines also supposed to apply to spec/test files? If I'm trying to keep methods under five lines, but also testing a heavy feature, it seems silly to break it out into two files and then cause confusion when one starts breaking.

Tmee commented 9 years ago

2 and #4 are where I slip up the most. #2 I am getting better at, but still find myself using methods that are longer than 5 lines. Mainly due to time now, refactoring is always a possible activity. #4 I did not know about, or remember, until reading the article. I have absolutely done this in most of my projects and could not think of a solution if you needed more than one piece of information inside a view. The example of what the author did with his team was a pretty cool trick, definitely cleans up how the controller reads.

dglunz commented 9 years ago

I've been following the 4 parameters and 5 line methods quite well. I agree with @dalexj about not being religious about abstracting too much to stay below 5 lines. There are times when I have to jump around to different methods and it just makes it more confusing to follow.

The rule about only having one object in the controller is difficult at times. I don't clearly see the benefit of the whole facade approach, so I guess I'll have to give it a try. It feels like one of those tricks where you're just moving shit around and not making it any more legible or efficient. But then again I haven't tried it so maybe it is helpful.

katelane commented 9 years ago

Welp, at least there is Rule 0.

gregnar commented 9 years ago

I have to say that I kind of like the facade approach, since it keeps your controller as lean as possible. So when at some point you have to change what you're passing into your view, you're not monkeying around with private methods et al in the controller and asking yourself whether or not you need to push stuff down the stack.

gregnar commented 9 years ago

To answer the question: I think the four parameter constraint would be the hardest to follow, were it not for Metz making an exception for view helpers. If you're linking to a POST action, you've already got three parameters swallowed up by the time you start adding html classes and ids.

stevekinney commented 9 years ago

@dalexj I think of them more as guidelines than rules. They ought to be broken when you have a really good reason. But, they should also serve as potential code smells. It's super easy to think—in the moment—that you have a really good reason to do something. But that usually leads to technical debt. Do I violate Sandi's rules—you're damn right I do. But, I think long and hard before I do. TL;DR It's a guideline, not a mandate.

@trayo It's kind of like I said above—in general I think that it's safe to skip the rule for test files. But you should at least think about it a little as you cross the 100-line mark, right? But yea, I think this most applies to code you write (granted, tests are code you write).

@katelane Rule 0 is arguably the most important rule.

@gregnar The facade approach is especially good for something like a dashboard. Especially, if you find yourself putting a tone of logic in the view. Abstract it out to a plain old Ruby object! One of of the things I think is really important in Module 4 is making the connection back to plain old Ruby to solve complicated problems in Rails.

chandracarney commented 9 years ago

I'm with Tim on the 'Controllers can instantiate only one object' rule being hard. I seem to remember Pivot posing this challenge with slugs and other instance variables in views, but revisiting with Dashboard would be good to do. I think we've all been trying pretty hard to follow these rules in general; these are nice to have fresh on the mind while launching into the 4th module.

chandracarney commented 9 years ago

Listening to that Sean Griffin and Derek Prior podcast now.

stevekinney commented 9 years ago

@Copywright and @zrouth Thoughts?

zRouth commented 9 years ago

I like these as guidelines, not rules. I think keeping classes below 100 lines is solid and I bet most of us have followed that once since our time at Turing. I like the idea of keeping methods no longer than 5 lines, but i imagine that following that rule to a T might prove to be more work than its worth, I could imagine having almost too many refactored methods and it might get a little cumbersome for the reader. The single action controller and the =<4 parameters to a method definitely seem like the hardest to follow. But all in all I can't imagine that if you keep these things in mind when working on a project that it would be a bad thing.

hjoseph96 commented 9 years ago

Oh, I read it but just saw these comments.

I like these as general guidelines, but I don't know how 100-line classes line up with Fat Models/Skinny Controllers. I agree with the short methods methodology, it's always made sense to me. Short methods are just easier on all those involved. Also, keeping them short seems to work well for never coming into contact with 4+ params for one method before.

The one object per action is interesting, I think that one is a tough one unless I use presenters. @stevekinney is it a thing to prepackage a single object for controller use in models? Like, on OpenStruct with all the needed data and that way you adhere to that rule?

stevekinney commented 9 years ago

Yea, one way to go about it is with the facade pattern.

http://tutorials.jumpstartlab.com/topics/models/facade_pattern.html