While running the code, I did find a bug where ships could overlap.

As I started trying to write a test for the ShipPlacer class, I found it was a little hard to do. There is a bunch of randomizing involved and because the primary thing that happens is updating the ship coordinate state, it was a little hard to test things in isolation.

I refactored ShipPlacer so it no longer directly owned any of the logic for generating coordinates for a random ship placement. The only thing this class now does is go through the ship placements provided by the _get_valid_ship_placements function and set the coordinates for the ship.

class ShipPlacer:

    def __init__(self, ship_configs, rows, columns, ship_coords, get_valid_ship_placements = get_valid_ship_placements):
        self.ship_configs = ship_configs
        self.rows = rows
        self.columns = columns
        self.ship_coords = ship_coords
        self._get_valid_ship_placements = get_valid_ship_placements

...

    def _attempt_placement(self, ship_coords, ship_config):
        for potential_placement in self._get_valid_ship_placements(ship_coords, ship_config, self.rows, self.columns):
            if potential_placement is not None:
                for coord in potential_placement:
                    ship_coords[coord] = ship_config['name']
                return True

        return False

This allows me to write a more simple unit test. I can control the results returned by _get_valid_ship_placements and verify it uses those values in the ship_coords.

                                                                                                                                                                                                                                              
    def test_valid_placment_sets_coordinates(self):                                                                                                                                                                                           
        config = {                                                                                                                                                                                                                            
            'columns': 5,                                                                                                                                                                                                                     
            'rows': 5,                     
            'ships': [                     
                {'name': 'B', 'length': 2},
                {'name': 'C', 'length': 2}
            ]
        }

        def placement(*args, **kwargs):
            ship_config = args[1]
            if ship_config['name'] == 'B':
                yield [(0,1), (0,2)]
            elif ship_config['name'] == 'C':
                yield [(2,4), (3,4)]

        placement_fn = Mock(side_effect=placement)

        placer = ShipPlacer(config['ships'], config['rows'], config['columns'], {}, placement_fn)
        ship_coords  = placer.build_ship_coords()

        self.assertTrue(ship_coords[('a', '2')], 'B')
        self.assertTrue(ship_coords[('a', '3')], 'B')
        self.assertTrue(ship_coords[('c', '5')], 'C')
        self.assertTrue(ship_coords[('d', '5')], 'C')

I can also write a unit test for the function that actually generates the possible ship placements and test it in isolation.

    def test_get_valid_ship_placements(self):
        ship_coords = {
            (0, 0): 'B',
            (0, 1): 'B',
        }
        ship_config = {'name': 'C', 'length': 2}
        placements = list(get_valid_ship_placements(ship_coords, ship_config, 2, 2))

        self.assertTrue(len(placements) > 1)

        # currently not deduping based on where we first placed the ship
        placement = placements[0]

        # ship can only be placed in the following cooords
        self.assertTrue((1,0) in placement)
        self.assertTrue((1,1) in placement)

Project changeset.