Project Ramon

A learning journey from a Ruby noob perspective

Embracing Change pt. 2: Enforcing Single Responsibility Everywhere

pooder2_header_img

Today I’ll continue my efforts in understanding the benefits of writing maintainable code. This post, as did the last one, is inspired by my reading of Sandi Metz’ book Practical Object-Oriented Design in Ruby. I have been covering books on design recently, with the goal of writing code thats easier for a human to both understand and change. In my next post I’ll be covering  a TicTacToe game, I’ll either post the unfinished program with a list of challenges left to overcome, or I’ll be refactoring a complete v1 TicTacToe program.

If there was a theme in chapter2 of Sandi’s book, one could make a case that it would be enforcing single responsibility everywhere.  Creating Classes and methods with a single responsibility makes them easier to change and reuse in the future, which can cut down on a developer’s technical debt.

Here’s an example pulled from Sandi’s book to kick off our single responsibility discussion.

This method violates the single responsibility principle. One responsibility of this method as it stands is to iterate over wheels, and the second responsibility is that diameters also calculates the diameter of every wheel in the wheels array.

If we wanted to refactor diameters into two separate methods, each with only one responsibility we could do something like so:

Sandi points out that separating iteration from the action thats being performed on each element is a common case of multiple responsibility thats easy to recognize. But there are other cases where extracting extra responsibilities from methods aren’t so obvious.

Due to the fact that the gear_inches method includes a calculation for a wheel’s diameter, we can extract this wheel.diameter calculation into a new diameter method which will contribute to an easier examination of this class’s responsibilities.

This refactor doesn’t change how a wheel diameter is calculated, instead it isolates the exact same behavior in a separate method. Here’s an excellent excerpt from Sandi describing the reasoning behind refactoring to follow SRP:

These refactorings are needed, not because the design is clear, but because it isn’t clear. You do not have to know where you are going to use good design practices to get there. Good practices reveal design.

Methods that have only a single responsibility provide the following benefits:

  • It exposes previously hidden qualities: Having each method serve a single purpose makes the set of things the class does more obvious.
  • It avoids the need for comments: If a bit of code inside a method needs a comment, extract that bit out into a separate method. The new method’s name will now serve the same purpose as the comment once did.
  • It encourages code reuse: Other programmers will reuse the methods instead of duplicating the code. They’ll follow the pattern you have established and create small, reusable methods in return.
  • It makes it easy to move methods into another class: With small methods, you can rearrange behavior without doing a lot of method extracting.

I hope that these past few posts have been as insightful for you as they have been for me. I’m looking forward to applying these new perspectives of thinking to some upcoming Ruby programs.

Here’s a view of a TicTacToe program that I’m working on completing. Can you point out places in need of single responsibility? Maybe its best for me to get it working first. 🙂

pooder2_img1

In either case (finished or still in progress), I’ll be covering this game in my next post.

pooder2_img3

Stay tuned!

Advertisements

Categories: Ruby

Tags: , ,

6 replies

  1. Hey Ramon,

    I’ve set up a repository on github, importing your initial code. And then applied a few refactorings to fix bugs as well as improving the design regarding ‘Single Responsibility’ rule. You can see the full history at:

    https://github.com/ambisoft/ruby-tic-tac-toe/commits/master

    Notes:

    – I think you mixed horizontal & vertical rules; horizontal would imply the 2nd coordinate stays the same; so I swapped them

    – if you do it and look at these coords in terms of x,y being different/fixed, you will see your current @vertical_winner – middle row – is wrong (should be [1, 0], [0 ,0] , [-1 ,0] i.e. 2nd coordinate fixed )

    – before changing anything I added Rspec to the project and started writing specs ASAP. Which I think is essential before doing any sort of refactoring

    – these coordinate-based rules are not really flexible either; you may have a player which puts his marks in, say, upper horizontal row, but instead of using [1, 0], [0 ,0] , [-1 ,0], he will do [-1, 0], [0, 0], [1, 0] . The current check would not catch that, because it operates on literal sequences.

    So instead, it’s better to define these rules dynamically – what makes a sequence horizontal, vertical or diagonal? That was my next change.

    Then finally we clearly have (at least) 2 things going on in the TicTacToe object: 1. it registers each move and then 2. determines the winner. Clearly it violates a single principle rule – so I extracted the WinnerRules which represents the ‘winning’ rules of the game. TicTacToe still contains the winner? method, but it simply delegates to THE correct place where the actual rules are defined.

    You should add checks against valid/invalid moves too – and probably it would deserve a separate class as well. With TicTacToe the rules are simple (any field that is empty, is available) – but the rules may change / get more complicated, so it’s good to encapsulate them.

    Finally the winner check is wrong right now; you should check it as soon as the move is made, right now it’s performed when the next player tries the move. So Player1 makes his winning move, then Player2 invokes the ‘play’ and sees the winner message – but it’s Player1 who really deserves the credit 😉

    Sandi advocates NOT using any class names as dependencies – so here TicTacToe should not have the ‘WinnerRules hardcoded. I’m not 100% sure this rule makes always sense, but in the constructor the TicTacToe object checks for the ‘rules’ object passed to it – and falls back to the WinnerRules if none was provided. This at least allows to do simple dependency injection (and for example pass a stub/mock rules object in the Specs)

    Hopefully it illustrates at least a few points. I’ve been recently working on a Rails backend for one boardgame (slightly more complicated than TicTacToe), so I’m ‘deep in the subject’ at this moment 🙂

  2. Hello Marek,

    I was very stuck as to why I wasn’t getting a winner when entering in what I thought were proper coordinates for a win into the play method. It was also extremely insightful to read over your specs.

    I currently don’t understand how your dynamic rules work, but that will be a fun project for me this evening and tomorrow. I plan on reading the docs and playing with PRY until I grasp what player_moves.map(&:first).uniq.size and player_moves.map(&:last).unique.size does. This will be good for me 🙂

    The design critiques you mentioned make sense, and I will definitely make the changes and highlight this also in my next post.

    I truly appreciate you taking time out,

    Ramon

  3. Hey Ramon,

    Glad that you find it helpful 🙂

    These ‘map’ clauses really illustrate the following questions:

    – what makes a sequence of moves, where each move is defined as a pair of [x, y] coords, be oriented horizontally i.e. being positioned in the same row, knowing that they are guaranteed to be unique (no pair may appear more than once – since it’s not possible to place anything on the field that already has been occupied before)

    – same for the ‘vertical’ orientation

    – the cryptic array.map(&:first) is really a short version ‘create an array from an array, taking 1st element from each element’ which can also be written as:

    array.map { |pair| pair.first }

    Example:

    [ [‘a’, ‘b’], [‘c’, ‘d’], [‘e’, ‘f’] ].map { |pair| pair.first }

    => [“a”, “c”, “e”]

    In general think about array.map(&:method1) as a convenient way to take some existing array (enumerable actually), invoke the ‘method1’ method on each element and create a new array from results of these calls.

    Once we can extract 1st (x) coords or 2nd (y) coords, we can then try performing the checks like:

    – how many distinct ‘x’ coordinates are used in a sequence (hint: vertical pattern means all moves have the same ‘x’ value)

    – how many distinct ‘y’ coordinates are used in a sequence (hint: horizontal pattern means all moves have the same ‘y’ value)

    – array.uniq.size returns number of unique elements in an array (or more precisely: size of a new array, which consists only of unique elements of the original array)

Trackbacks

  1. My TicTacToe Failure: Airpair Mentor to the Rescue | Project Ramon
  2. Ruby TicTacToe: The Unravel | Project Ramon
  3. Ruby Refactoring: Substitute Algorithm | Project Ramon

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s