6
\$\begingroup\$

I am trying to code Battleship. It should be a text one-player game against computer where computer and human player take turns in shooting at opponent's ships. I decided to start implementation with this part: A human types target coordinates and computer answers if a ship has been hit or even if it has been sunk. Ship positions are fixed (predefined in the program). For some time I wondered how to represent game plan and ships to get this working as smoothly as possible. This is what I have put together:

class GamePlan:

    DIMX = 10
    DIMY = 10

    SHIP_MISS = 0
    SHIP_HIT = 1
    SHIP_DEAD = 2

    SHIP_COUNTS = {
        2: 1,
        3: 1,
    }

    def __init__(self):
        self.ships = [
            [(1,1), (1,2)],
            [(5,6), (5,7), (5,8)],
        ]

    def hit(self, target):
        hit_ship = None
        hit_ship_index = None
        for i, ship in enumerate(self.ships):
            if target in ship:
                ship.pop(ship.index(target))
                hit_ship = ship
                hit_ship_index = i
        if hit_ship == []:
           self.ships.pop(hit_ship_index)
           return self.SHIP_DEAD
        if hit_ship:
            return self.SHIP_HIT
        return self.SHIP_MISS

def main():
    game_plan = GamePlan()

    while True:
        raw_coords = raw_input('Enter coords: ')
        str_coords = raw_coords.split()
        coords = tuple([int(c) for c in str_coords])
        if len(coords) != 2:
            print 'Bad input'
            continue

        result = game_plan.hit(coords)
        if result == GamePlan.SHIP_DEAD:
            print 'Ship dead'
        if result == GamePlan.SHIP_HIT:
            print 'Ship hit'
        if result == GamePlan.SHIP_MISS:
            print 'Missed'

if __name__ == "__main__":
    main()

EDIT: GamePlan should be probably called Board as answer of Janne Karila suggests. Just to clarify what I meant by that name with my flawed English.

There are a few things I am unsure about:

  1. Is it correct that GamePlan processes shooting (in the method hit)?

  2. Is hit a good name for that method or should it be something like process_hit? Because GamePlan is being hit, it is not hitting anything. Is hit still good in such case? This is probably my biggest concern.

  3. Should ships be represented as objects of a class Ship instead?

  4. Is GamePlan a good thing to have or is it useless? I mean I could make class AI that directly owns ships but I am not sure where stuff like SHIP_COUNTS would go then. I have also planned that I will use GamePlan to generate ship positions (hence SHIP_COUNTS and DIMX, DIMY which are unused atm) but plan generation could also easily go to AI, I guess.

  5. Is there anything else that is wrong?

\$\endgroup\$
2
\$\begingroup\$

Perhaps GamePlan would better be named eg. Board? English is not my mother tongue, but AFAIK game plan == strategy.

1,3,4: The ships are static, they just occupy some space on the board. However, a ship object would be aware which squares belong to the ship, and perhaps should be responsible to determine if it has been sunk.

2: hit is perfect; in OOP the object is often the object also in grammatical sense. Think of file.close().

5: If the player hits the same coordinates twice, you'll report a miss, which is wrong.

\$\endgroup\$
  • \$\begingroup\$ Thank you! Especially for that point 2. I was thinking many hours about that (feeling stupid that I think about something like that) and couldn't even quite formulate what I was thinking about. Now I can thanks to your remark about grammatical sense:). Specifically, whether programmatic objects are objects of operation when a method is called upon them or if they are subjects and method names should reflect it. I program for pretty long time but today my head got messy. \$\endgroup\$ – clime Apr 26 '13 at 20:11
  • \$\begingroup\$ at 5.: Well this what I wanted. It is miss because that part of ship is already destroyed. Is it played differently? \$\endgroup\$ – clime Apr 27 '13 at 14:31
  • \$\begingroup\$ @clime I would imagine that the ship remains floating until it sinks. Of course, it's a mistake on player's part to hit the same spot again, as it gives no new information. \$\endgroup\$ – Janne Karila Apr 27 '13 at 19:41
2
\$\begingroup\$
  1. I think it is good idea to have method hit inside GamePlan because you don't plan to use it in other place then GamePlan I assume.

  2. You have chosen good name for method hit. 'Simple is better then complex'.

  3. If you plan continue develop game you suppose to put ship into separated class it will be easier to operate when code will grow.

  4. PlanGame is good idea to have. It helps separate game code from future Menu for example. What you can do change PlanGame to GameController. The GameController could manage all objects and get orders to do such as .hit(), .create_ship(), .restart() etc. Also this will let you create Plan or Map which will be managed by GameController also.

  5. I cannot see any docstrings. Remember that we read code more often then write.

Your result codes are good but you can enhance them:

SHIP_MISS = 0
SHIP_HIT = 1
SHIP_DEAD = 2

RESULTS = {SHIP_MISS: 'Missed',
           SHIP_HIT: 'Hit',
           SHIP_DEAD: 'Dead'}

And now you can do:

if result == GamePlan.SHIP_DEAD:
    print GamePlan.RESULTS[SHIP_DEAD]
if result == GamePlan.SHIP_HIT:
    print GamePlan.RESULTS[SHIP_HIT]
if result == GamePlan.SHIP_MISS:
    print GamePlan.RESULTS[SHIP_MISS]

or even:

print GamePlan.RESULTS[result]

This is closing for changes and opening for improvements try do that as much as possible.

\$\endgroup\$
  • \$\begingroup\$ Thank you. That dictionary in 5. is cool. And GameController is very interesting idea in 4. \$\endgroup\$ – clime Apr 26 '13 at 20:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.