turingschool-examples / intermission-assignments

33 stars 218 forks source link

Sandi Metz's rules for developers #45

Closed biglovisa closed 8 years ago

biglovisa commented 8 years ago

Discuss Sandi Metz's Rules for Developers here. Which of Sandi's rules do you feel like might be the hardest to follow—why?

acareaga commented 8 years ago

I think Sandi’s rule of instantiating one object in the controller would be the hardest to follow, at least when starting a project. I feel like I’d lose sight of the “goal” by overdesigning first. It makes a lot of sense in refactoring though.

amcrawford commented 8 years ago

Sandi's rules really resonated with me though some of them seem a bit challenging. I loved that she is emphasizing really clean and clear code overall as well as single-responsibility. I definitely strive for this when refactoring though, I've found her 5-line-method rule to be one of the hardest to keep, particularly in the case of using if-else statements. Sandi stated that, "Five lines ensured that we never use else with elsif", which I have definitely done in the past. I will try to keep this in mind in the future and look out for best opportunities to pull logic elsewhere.

jillmd501 commented 8 years ago

In Module 1, I was happy to just be able to make a project work (ahem Mastermind). When I heard instructors mentioning this term "refactoring," I would scoff at them and continue writing my 25 line methods. From my sad glimpse of my Module 1 work, one could assume that the 5 line method rule would be hardest for me to work with. I am learning about pushing logic down the stack and have implemented it in projects, but this still is one of the hardest parts of programming for me.

MattRooney commented 8 years ago

I think the rule of instantiating only one object in a controller if the most difficult because it's a little counter intuitive at first, when you're simply trying to provide your view what it needs. But I think all of these rules make more sense and become easier to use as you step back and realize they all work together. It's all about keeping things as simple as possible, even if it seems less than simple at first. I think it's ok to approach these rules as goals, and to be ok with breaking some of them on the first iteration and then working towards them when refactoring.

adamki commented 8 years ago

I found the following rules bit restrictive:

Rule 3: Pass no more than four parameters into a methods.

If this pertains to class methods, I believe this is fine and easy to follow. Personally, I have not been pressed to keep class methods within 4 args. However, as pointed out in the article, some view helpers just require a LOT of args. Just take a look at the docs for some things like f.collection_select. Good luck keeping that under 4 args.

Rule 4: Controllers can instantiate only one object.

This was much easier to follow when building Simple Rails CRUD apps. Every View is dumb and knows of only a single Model. The User will never mingle with the Item in the same view. However, When attempting to build more useful Web Apps that have something like a dashboard(where a user can manage something like Orders, and search for new Items to purchase) this becomes more difficult With Rails convention. I've repeatedly struggled with "correct" ways to tell Rails that I want to have access numerous models in a single controller. I feel like Rails fights and is stubborn to let this happen.

I do like this article for providing helpful partial examples as workarounds. I have also began to reach out to other methods, such as Internal API endpoints to access data. However, to me, this Rule seems the hardest to keep.

jasonpilz commented 8 years ago

The rule that appears to me as the most challenging would be limiting every method to 5 lines. Especially considering control-flow constructs as taking up lines. At the same time, I love the idea of this rule because it would force a natural cohesion of single-responsibility methods. Also, I loved the example of the 'facade pattern' for instantiating one object in controllers.

matt-stj commented 8 years ago

I could see myself having a hard time implementing the following rules:

  1. Methods can be no longer than five lines of code.
  2. Pass no more than four parameters into a method. Hash options are parameters.

I understand how keeping methods short can keep your code clean, but there are cases where slightly longer methods could be just as helpful/readable. When the Thoughtbot blog mentioned "Five lines ensured that we never use else with elsif", it forced me to think how I would tackle writing methods that rely on multiple conditional statements. I would probably prefer an 7 line method with an else if + else statement if needed.

As for only passing four parameters into a method, there have been multiple occasions where I've had to break this rule -- most often with things like 'link_to' helpers. It's likely there were ways around this, but it would've required restructuring significant portions of my apps to adhere to this rule.

Overall, Sandi's rules for developers were really insightful and I'll keep them in the back of my mind as I move forward with future coding projects.

rossedfort commented 8 years ago

After reading the article by Caleb Thompson on how he implemented Sandi's rules with his team, I tried to find instances of when I had broken these rules in past projects. I thought Little Shop was a great example, more specifically, the cart functionality. There were several small methods we needed to write in order to give our cart the functionality it needed, such as total price, total items, etc. However when writing these methods on Models that have complex database relationships, the lines blur on which Model the methods should actually be called on. It's reasons like these that cause me to sometimes struggle with 100 line class/single responsibility rule. In a complex application, the opportunities to break this rule seem to increase.

Another rule that I think would be difficult to follow strictly is that controllers can only instantiate one object. In my personal project, I had PORO 'models' that talked to a service, which talked to an API. It was difficult to determine what model I should be calling methods on, given that the data I was retrieving was related to both users and repositories. Originally I had all my methods contained in a User object, but refactored some out to use a new Repo object. As a result of this, I was calling methods on both User and Repo in the same controller. I then created a corresponding repo controller, and it cleaned up my code quite a bit. In retrospect, I think it was a good choice to refactor in that way.

In conclusion, I see Sandi Metz's rules for developers as Captain Barbossa sees the pirate code pirate-code

sekharp commented 8 years ago

I share a lot of the same thoughts as Jason's response above. I thought the method rule would be hardest to follow, in a variety of cases. The conditionals that Sandi Metz called out as best having only a single if/else (no elsif statements allowed) would mean I would need to refactor every other conditional I've written in my past projects :) But I get the impetus - the rule really has you break out single responsibility portions of functionality, name them, and have eventual methods that descriptively flow through what's happening in the code. So I see where it comes from and will try to emulate that in my code.

On the flip side, while I initially had trouble with the controllers rule, of passing only one instantiated object on through to a view, I've done that on a handful of projects and loved how natural it now feels. It's a habit that keeps my mind focused on collections of material and data that gets passed around and manipulated, as opposed to constantly getting caught in crossfire and confused managing 3-7 variables in a poorly written controller/view.

joshuajhun commented 8 years ago

https://gist.github.com/joshuajhun/e5e966c1ceb5168a94c1