Making my Ruby 'Legacy Code' SOLID
Tags: ruby, solid
Published: 2020-04-07 10:57:57

Several weeks ago I was working on a TicTacToe game in Ruby, but then I moved on to Java for a while. Now I've returned, full of type-security and  compiler error handling, and hopefully some transferrable things too. There's plenty to look at, but I'll just be working through a single example for each SOLID principle.

**Dependency Inversion**

I'm starting with the 'D' in SOLID because when I came back to this project I noticed how tightly coupled the whole thing was, making any change very difficult. Most classes instantiated others instead of having them injected, so that was the first thing I changed.

![Small knot](https://66.media.tumblr.com/3974481146edff7cad56eefdc0c81813/tumblr_mxtcllhifT1qls3mfo1_250.gif)


Take a look at this test for my `Game` class. It makes an instance of `Game`, refers to both `Board` and `ConsoleInOut`, and fixes the `gets` result of `ConsoleInOut`.

**Before**

    require 'game'

    describe Game do
        it 'Asks for board size input' do
            allow_any_instance_of(ConsoleInOut).to receive(:gets).and_return('4')
            game = Game.new
            expect(game.board.board_size).to eq 4
        end
    end

How do we know that `Game` contains instances of these classes? How do we change their input and output? We don't and we can't (unless you count *allow_any_instance_of* as changing input).
We have an opaque bundle of prima-donna classes that insist the tests be written around them. Let's change that by instantiating all of `Game`'s dependencies externally and then injecting them on instantiation.

**After**

    require 'game'

    describe Game do
        before(:each) do
            @inOut = ConsoleInOut.new('', '')
            @player_x = Player.new('Player 1', 'X')
            @player_o = Player.new('Player 2', 'O')
        end

        it 'Asks for board size input' do
            board = Board.new(@inOut, 4)
            game = Game.new(@inOut, @player_x, @player_o, board)
            expect(game.board.board_size).to eq 4
        end
    end

Now we know exactly what `Game` contains, because we passed them in, and this allows us to change the parameters of these classes beforehand. For example, when we pass in a `ConsoleInOut` instance, we can replace STDIN with a string to simulate input, and STDOUT with a string so that the test output isn't polluted by every print method.

**Single Responsibility**

Classes shouldn't have more than one responsibility, plain and simple. There are more technical explanations out there, but it's enough so say that my `ConsoleInOut` class is both getting input and validating it.

It works, but if I were to swap out `ConsoleInOut` for, say, `WebInOut`, I'd need to reimplement validation as well as Input/Output.

Since `Game` will always get the same input, there's no reason to ever change validation, so let's go ahead and put it in the `Game` class that handles interactions between classes.

**Before**

    class ConsoleInOut
        def initialize(input, output)
            @input = input
            @output = output
        end

        def get_input
            #
        end

        def print(to_print)
            #
        end        

        def select_move(max_turns)
            print("Please enter a free number from 1-#{max_turns}")
            player_input = gets.chomp.to_i
            valid_player_input(player_input, max_turns)
        end

        def valid_player_input(player_input, max_turns)
            player_input >= 1 && player_input <= max_turns ? player_input : 
            select_move(max_turns)
        end

        # And some similar select/validate methods
    end

    class Game

        # And the rest of the Game

        def submit_move
            player_input = @inOut.select_move(@board.max_turns)
            @board.make_move(current_player.mark, player_input - 1)
            @board.view_board
        end
    end

**After**

    class ConsoleInOut
        def select_move
            player_input = gets.chomp.to_i
        end

        # Remove valid_player_input, rest is the same
    end

    class Game

        # The rest is the same

        def submit_move
            valid_move = false
            until(valid_move)
                @inOut.print("Please enter a free number from 1-#{@board.max_turns}\n")
                player_input = @inOut.select_move(@board.max_turns)
                valid_move = @board.is_square_free?(player_input - 1)
            end
            @board.make_move(current_player.mark, player_input - 1)
            @board.view_board
        end
    end

Now `ConsoleInOut` just gets input for `Game`, without knowing or caring what happens next. `Game` is now a bit bigger, but more complete, and validation is functionality that shouldn't ever need changing. As a bonus, if anything goes wrong with input and validation, it's easier to place the blame.

![One job](https://media1.tenor.com/images/924d33230b63461a5bfa6dc70b633d73/tenor.gif)

**Open/Closed**

I'm going to be very daring and use a metaphor now:

- A gaming console or desktop computer is complete in itself.

- It doesn't need a particular screen to work, just *a* screen, and the HDMI port lets you plug it into a huge range of screens without changing anything else.

- If one screen breaks, the console still works, just find a new screen.

- But if you took out the port and built-in a screen, it would be less modifiable, and the screen breaking would be serious.

Your code should have more 'ports' and fewer 'built-ins'. Metaphor over, promise that's the only one.

**So...**

In the case of a program, it should be be complete and closed to changes, yet open for new features to be added.

If I were to add a Computer Player to my game, it would require rewriting the game so that the Computer could choose to make a move without console input.

In order to make the program open to extension, it is necessary to create a seam by moving the responsibility of selecting a move to the `Player` class. Then, the core program will not require changes.

Take the code above as a 'before' example for `Game`.

**Before**

    class Player
        attr_reader :id, :mark
        def initialize(id, mark)
            @id = id
            @mark = mark
        end
    end

**After**

    class HumanPlayer
        attr_reader :id, :mark
        def initialize(id, mark, inOut)
            @inOut = inOut
            @id = id
            @mark = mark
        end

        def make_move
            @inOut.select_move
        end
    end

    class Game

        # The rest is the same

        def submit_move
            valid_move = false
            until(valid_move)
                @inOut.print("Please enter a free number from 1-#{@board.max_turns}\n")
    ----->      player_input = current_player.make_move
                valid_move = @board.is_square_free?(player_input - 1)
            end
            @board.make_move(current_player.mark, player_input - 1)
            @board.view_board
        end
    end

Now that the instance referred to by `current_player` is responsible for returning a move, we can just swap out a now `Human_Player` class to extend the program with a Computer player. See below.

**Liskov Substitution**

In Java, you can make sure that one class is substitutable for a similar one with interfaces or abstract classes. Ruby doesn't have that, but as a colleague is fond of saying, "we can keep the interface in our heads". An interface contains a list of methods that a class doing this particular job must have, but leaves the implementation up to each class. So instead of having an interface file and having the compiler enforce it, we will just imagine that that's going on.

See the `HumanPlayer` class and its tests below to see what we're working with.

**Human Player**

    class HumanPlayer
        attr_reader :id, :mark
        def initialize(id, mark, inOut)
            @inOut = inOut
            @id = id
            @mark = mark
        end

        def make_move
            @inOut.select_move
        end
    end

**Tests**

    require 'human_player'
    require 'console_in_out'

    describe HumanPlayer do

        before(:each) do
            inOut = ConsoleMock.new('3', '')
            @player = HumanPlayer.new('Player 1', 'X', inOut)
        end

        it 'Stores player name' do
            expect(@player.id).to eq('Player 1')
        end

        it 'Stores player name' do
            expect(@player.mark).to eq('X')
        end

        it 'Can make a move' do
            expect(@player.make_move(9)).to eq(3)
        end
    end

The `HumanPlayer` class stores information (id and mark), and has a `make_move` method. If this class implemented an interface, that interface might look like this. 

**Player Interface**

    interface Player
        def initialize(id, mark)
        end

        def make_move(max_turns)
        end
    end

With the same methods, but no body, you can implement the functionality however suits you, and replace a generic `Player` instance with any instance that fits this pattern.

I've also added `max_turns` to `make_move` as I'm anticipating the computer using the `rand` method in the initial implementation, so `HumanPlayer` needs this to make the classes substitutable.

Note: It's worth passing in an instance of the `Board` class so that the computer has knowledge of the Board state, but for the sake of this example I'm keeping it short.

**Computer Player**

    class ComputerPlayer
        attr_reader :id, :mark
        def initialize
            @id = 'Computer'
            @mark = 'O'
        end

        def make_move(max_turns)
            rand(max_turns)
        end
    end

To summarise, it doesn't matter how `Player` or `ComputerPlayer` work, just that they contain the same core methods. Want to add methods to one or the other? Fine, they will still conform to the Liskov Substitution Principle as long as they both have `make_move` and store id and mark, since other classes refer to these.

If I want to change how `HumanPlayer` or `ComputerPlayer` makes a move, I can do this without modifying `Game`, and I can add a third option without changing *anything*.

We're nearly done.

**Interface Segregation**

*I don't have a specific code example for this principle, as this is a small program with two (imaginary) interfaces. The 'imaginary' is important; Ruby doesn't have interfaces, so if the idea is still unfamiliar to you, feel free to move on to the next section.
Otherwise, I will briefly cover the idea behind the Interface Segregation Principle.*

The Interface Segregation Principle states that more, smaller interfaces are better than fewer, larger interfaces. This is to prevent the forced implementation of unused methods by a client. In the case that the In/Out interface grew too large, it may be possible that a class implementing it didn't use all of its methods, but had to add them anyway to avoid compiler errors. Why should you add code that isn't used? You shouldn't, meaning this bloated interface should be broken up so that each class only contains methods it uses.

It's also helpful to have smaller classes and interfaces to follow the Open/Closed Principle mentioned earlier; with fewer modifications necessary and more seams available, any future change is much easier.

**It's All Connected**

Interestingly there's overlap between the SOLID principles, so each of these improvements doesn't just improve the program in one way; having a Liskov Subsitution approved In/Out interface makes dependencies more manageable, as well as making the program open to extension. Furthermore, exporting `make_move` functionality to a `Player` better follows the Open/Closed Principle, narrows the responsibility of `Game`, and paves the way for a Liskov Substitutable `computer_player` class.