← Back to Upcase

Sandi Metz's Rules


(Upcase ) #1
In this episode, Ben and Joe discuss Sandi Metz's rules. You can read the summarized rules. Pull 1 Pull 2 Pull 3 Reek
This is a companion discussion topic for the original entry at https://thoughtbot.com/upcase/videos/sandi-metzs-rules

(Nicholas Henry) #2

This is a little off-topic, but I’m interested in the instantiation of the Review facade demonstrated in this screencast. Can you please describe the approach with the dependencies method used here.

Also I assume the exercise, revision etc are instance methods on the controller. Is that correct?


(Andy Waite) #3

Thanks for the discussion!

I’m very interested in the concept of using automated ‘rules’ to improve code quality. In fact, I recently set up ContinuousQuality.org as a resource for this, and have also made some contributions to the quality gem.

At the company I work for (Blacklane in Berlin) we have team of 6 developers, with a mix of levels from junior to senior. We started a new project recently and decided to use Reek from the very beginning, alongside RuboCop and rails_best_practices. (Co-incidentally, the team lead is the maintainer of Reek, Timo Rößner.)

It’s been an interesting journey so far. There was certainly some initial frustration about the rules imposed by Reek. Writing very short methods was a big change for most of the team.

We have our CI system configured to fail the build if it doesn’t pass the quality checks. I know this is controversial, but it’s been very effective so far. It brought up some important discussions about OO design and refactoring. One of the first useful outcomes was to introduce a Parameter Object.

We break the rules when it’s appropriate to by adding exclusions to Reek’s configuration file, but only after careful thought and discussion.

It was also interesting to see you’re using pattr_reader which I assume is from attr_extras. I’m often frustrated by writing boilerplate initiallizers and protected attr_readers, but I wasn’t sure if using something like pattr_reader is straying too far from idiomatic Ruby.


(Jon Seidel) #4

FYI… the three “links” on the video page at https://thoughtbot.com/upcase/videos/sandi-metzs-rules (Pull1, Pull2, Pull3) are obviously placeholders that go nowhere and should be removed.

Thanks for a great session!


(Andrew Broman) #5

@JESii, I believe those links are to pull requests within the upcase repo on Github. You might need to give the Thoughtbot folks your Github username to get access to it.


(Brandon Newton) #6

In one of the Weekly Iterations it was said to keep method arguments to a maximum of 4? Doesn’t this dependency class break that rule?