turingschool-examples / intermission-assignments

33 stars 218 forks source link

Sandi Metz's Rules for Developers #75

Closed rrgayhart closed 8 years ago

rrgayhart 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?

s-espinosa commented 8 years ago

2 would likely give me the most difficulty. I'm not positive that eliminating else if is a positive. Some decision trees aren't binary, and following this rule in the face of that seems to encourage being clever (by creating an additional method to check related conditions?), but not necessarily clear or concise.

notmarkmiranda commented 8 years ago

I think #4 would give me problems. Only instantiating one item per controller seems tough, especially when the view is something like a dashboard. passing that responsibility to the view seems counter-intuitive with MVC single responsibility.

I do like the facade pattern shown in the article, but it seems like a round about way to just follow the rule. Adding 12 more lines of code in the model in order to prevent 4 lines that pass 4 more instance variables.

chadellison commented 8 years ago

Number 4 would be the rule I would probably end up breaking most often. It seems like a good principle to follow. The dashboard object used as the example is a nice way of handling the various objects related to the user; however, it's difficult to discern how broadly this rule should apply. It seems less simple to create a third object to accommodate the interface of two related objects simply to avoid instantiating both of those objects in one controller. On the other hand, having to pass more than one object into a view might be an indicator that the design is not as good as it could be. A dashboard object more accurately serves the purpose of a dashboard view than a user object. Since a dashboard has properties that a user does not, perhaps a dashboard view warrants a dashboard object. I think it does not make sense to introduce an object just to accommodate the rule, but perhaps the need to instantiate multiple objects indicates that the design can be improved.

thomschlereth commented 8 years ago

I think number 3 would be the most difficult, followed closely by numbers 2 and 4. The thing is, there are work arounds for numbers 2 and 4. They might be a pain to implement at times, but very doable. Number 3 however I break all the time in hash form. Making an api call and passing all of it's info through to instantiate a new class. In the views with all those helpful helpers rails provides, which you're nearly breaking rule 3 right out of the gate as the article talks about. It's probably the rule that I will never bother trying to follow without knowing the reason why I should.

ghost commented 8 years ago

https://gist.github.com/Salvi6God/68576f05d39dfbd42316357bf2ebbebb

patrickwhardy commented 8 years ago

I don't know if anyone writing a big rails app could uphold #2. I immediately think of a create method in a controller. First we need a new object (1 line), then we need to check if it saves with a conditional (3 lines, - if/else/end), we need to create some flashes depending on the outcome (2 lines) and we need to redirect the user in each case (2 more lines). Perhaps this could be broken into helpers etc, but I'm not sure I see how this would improve such a method. Controller methods do seem like special cases and this may apply better to POROs or models where we don't have such restricted access to things like flashes and redirects.

kamiboers commented 8 years ago

Rules 3 and 4 initially seem to be more demanding, because they require structural workarounds - they would require the developer to rethink their approach to a problem and reconstruct their code in a way that minimizes the logic occurring at the higher levels of the application. But, I'm willing to bet that in cases where 3 and 4 were problems, there are underlying problems with the structure of the application that need to be addressed (though I'm not sure if I agree with 4).

I think that Rule 2 would be broken most often for legitimate reasons. Rule 2 should encourage refactoring of methods and drive us towards the single responsibility principle, but it would create more convoluted code for if/else clauses and case statements. The resulting code would be more complicated and less clear and readable for the developer.

theonlyrao commented 8 years ago

I have a hard time not thinking all of these rules are dumb. That's especially my reaction to rule 2 - I just still don't really see the problem with a few if/else/elsif in controllers. Sometime I like to see things not get handed off to other objects because it makes it easier to get the big picture of what's happening with the code.

And then I have to remember that I don't actually know anything yet and that I should probably just be quiet and listen to someone who does.

The one that would be the hardest for me to follow is 4 - allowing a controller to only instantiate one object, and letting a view only work with one instance variable at a time. Some views are working with data from many different classes, and for those views it seems the most straightfoward implementation to just hand multiple instance variables to the view. In the example, I'm not sure it would really help me when dealing with that code to be looking at four different partials for one page.

alirezaandersen commented 8 years ago

I have to agree with Ashwin, I feel like all of these rules are pointless and just another hurdle for programmers trying to implement a good design.

I feel like rules 2 and 4 would be the hardest to follow. With rule 2 the strict content of conditionals would force a person to make multiple methods(some private) and even at that point may make the code hard to follow for someone to else since the reader needs to jump around to see what the main method does.

Rule 4 is probably the hardest of all the rules. The dashboard object used as the example demonstrates a good handling of various objects related to the user however it seems a bit difficult if the view is working with data that would be used in multiple different classes. I do believe in single responsibility but I feel that if a view handles a particular form that has multiple different classes then the responsibility of that view is still considered single responsibility as would the controller. The separation of responsibility then should get divided up into the classes in which that data goes to. Rule 4 makes it seem like one extra step which is doable just pointless.

kristindiannefoss commented 8 years ago

https://gist.github.com/kristindiannefoss/3cfd7d0a1817e86b4870f08ea8500907

jeneve commented 8 years ago

I had thought that the rule about sending only one instance variable into a view was the hardest, but the poster's solution- making the 'dashboard' a PORO that knows everything it needs to, is great- why didn't I think of that? I now think that the 'methods should be less than 5 lines' is the hardest, and that flow control is the reason. I have heard about a solution wherein the various outcomes are stored in a hash and accessed by the flow control method, but in those rare cases where there may be more than 5 possible outcomes- this fails too (or it fails the 80 char line limit). But what if the hash was defined as a constant? Does the 5 line rule apply? The rule about classes not exceeding 100 lines is one that I have never really tested- when classes get long I often pull methods out into a helper module, but I don't think this counts. Generally I need more practice refactoring my code. I am more often lacking the time to refactor than I am lacking ideas of HOW to refactor; I don't think I'll truly know which rules are actually the hardest to follow until I try!

GKhalsa commented 8 years ago

I think rule 4 would be hardest to follow. However being forced into using a facade pattern or a presenter would really help organize your code, as well as give you the possibility to write whatever custom methods you need.

drew-t commented 8 years ago

2 and 4 seem to be the hardest to always follow, but 4 is the one that would be most commonly broken for me.

I think it's relatively easy to generally keep your methods short and occasionally break the rule if needed. However, I think that always passing only one object is difficult. I believe I have been told before not to use a hash to achieve this either, but I don't see where in practice a hash is different than an class instance with all of those things. Especially when thinking about javascript and the fact that an object there is very similar to a hash anyways.

jwashke commented 8 years ago

I think #4 would be hardest to follow out of all of the rules, but I really like the example they gave using a facade. I'm excited to try using that pattern in my next rails project.

3 can be difficult because sometimes you just end up needing to pass a big hash into something and assigning all of its values to variables, but thats pretty rare.

2 is my favorite rule because it gives me an excuse to use multiple guard clauses over long if else blocks

Claudia108 commented 8 years ago

Number 1 seems the easiest to follow. Number 2 seems hard to follow and the code might actually get more confusing when you have to find and make sense of the private or helper methods that get passed in to keep it short and concise. Number 3 seems hard as well. Sometimes you need to pass hashes, or when extracting repeating methods into helper methods you need to pass in more parameters (More than 4 seems very rare though in this second case). Number 4 seems doable using presenters, but I'm not sure if it always helps with clarity. In general I think it is good to have guidelines to keep oneself close to, but my general question usually is what is more useful and clearer for others: short concise methods with lots of helper methods, single object per controller or longer methods and more objects per controller and things are more in one place and easier to follow.

marinacor1 commented 8 years ago

In the limited experiences I've had with Ruby code, since my start in December, I've been able to more or less abide by the first three rules. However, the last rule was the hardest for me to follow.

I guess I would need to see more examples of this, but if a controller can only instantiate one object, and views can only know about one instance variable, how do you pass multiple nested variables? Thinking back to Mix Master in Module 2, where there were artists and songs, even with the relationship created, it made sense to define @artist and @song . I also think back to the work done with the Best Buy API, and how I needed to account for the zip store and count. It seems based on the rules that it would not be okay to assign three instance variables OR create some sort of relationship within the view to find them (if I understand correctly). This seems tough to figure out.

Anyway, I look forward to understanding this more fully and practicing this.

lingtran commented 8 years ago

https://gist.github.com/lingtran/87bac10feb3f0fc330ee83544f7b2ee4

Jbern16 commented 8 years ago

I think number 2 would e the hardest to follow. My experience on my personal project led me to believe that sometimes, there just is necessary logic that needs to happen. By limiting methods to 5 lines without context, can create confusing code if the parts extrapolated are not clear. At that point, it seems as if you are just moving code around for the sake of moving code around. I think until you see the code written its tough to see if there are opportunities to shorten it, logically.

pindell-matt commented 8 years ago

For me, rule #4 would prove to be the most trouble - as it's absolutely the rule I break the most often. While it would help in the pursuit of the other rules (e.g. shorter methods / classes, passing around fewer arguments) I also wonder if the trade-offs are always absolutely worth it. The extra lines of code to write in order to keep the balancing act must, at some point, make this rule not 'worth it' on some level. That is also the 2nd reason why I feel like this would be difficult, because it'd be the one I'd be most often tempted to break - likely spending inordinate amounts of time wrasslin' with the choice to break or adhere.