Battleship algorithm

Im looking to improve my search algorithm in my battleship game. Code is not perfect but would appreciate any suggestions or recommendations.

Running the simulation using a 100x100 grid (10,000 possible locations) Im averaging ~30% accuracy.

Games played 500 Average = 0.31108

Im using a forward and backwards diagonal search approach. The search diagonal locations by moving forward and backwards by the using the maxShipSize that is still active. As a ship is sunk the maxShipSize changes.

If a ship is hit i check a cross pattern (left,up,right and down). When a ship is hit more then once a sequential pattern search occurs.

battleship.py

```
# -- coding: utf-8 --
from __future__ import division
import numpy
import operator
import commands
import random
import sys
import os
from time import sleep
from grid import grid
from fleet import fleet

#setupNavy
#Purpos: To place our ships on the grid
#Receives: setupSelection- either manual or random
# gridClass
# sortedShipList
#Return:
def setupNavy(setupSelection,gridClass,sortedShipList):
used="no"
shipCoordList=[]
for shipData in sortedShipList:
shipName=shipData[0]
shipSize=shipData[1]
if setupSelection=="manual":
header= "Ship placement: %s size is: %s\n"%(shipName,shipSize)
sys.stdout.write(header)
con="yes"
while con=="yes":
if setupSelection=="manual":
start=raw_input("Enter start point: ")
else:
randomX=random.randrange(0,gridClass.xGridSize)
randomY=random.randrange(0,gridClass.yGridSize)
xLetter=gridClass.alphDict[randomX]
start="%s%d"%(xLetter,randomY)
coordStatus=gridClass.checkDataPointValue(start)
if coordStatus=="E":
con="no"
if setupSelection=="manual":
placement=raw_input("Place Vertical (V) or Horizontal (H): ")

Some general style suggestions; incomplete due to lack of time, but they should give you some things to think about.

Upgrade to Python 3 if possible. You appear to only be using stdlib code, so this should be reasonable painless.

You have a lot of java-esque naming conventions. Python style (see especially PEP 8) prefers methods and variables to be named like_this rather than likeThis. Names of classes should be capitalised, so Grid instead of grid. You have some of your methods named according to this style already - you should generally try to pick one style and stick with it; preferably follow the conventions in PEP 8 unless there's a compelling reason not to.

You use strings as flags a lot - eg,

used="no"
con="yes"
while con=="yes":

Python supports booleans, so you can do

used = False
con = True
while con:

Comments like this:

#setupNavy
#Purpos: To place our ships on the grid
#Receives: setupSelection- either manual or random
#          gridClass
#          sortedShipList
#Return:

Should be docstrings - basically, make it an unassigned string literal, and put it directly inside the function definition.

The first argument to setup_navy is another string-as-flag. Use a bool instead, and all it automatic_placement. But even better, split this into two functions. The one that does it automatically could actually be a method of Grid called place_ships, and document it as placing them randomly. The one that places them manually does belong out in the main flow of your program, since it continually does IO. This will simplify your code greatly.

The second argument is called gridClass but actually expects a Grid instance. Just call it grid.

sortedShipList could just be called ships. If you want to document its structure (which isn't a bad idea), you can do better than giving it a clunky name that only documents some of it's structure - in your docstring, do this:

def place_ships(self, ships):
    '''
    Randomly place the given ships on the grid.

    `ships`: a sorted list of tuples (name, type)
    '''

Consider using a namedtuple for the ships, so then this loop:

for shipData in sortedShipList:
    shipName=shipData[0]
    shipSize=shipData[1]

can become just:

for ship in ships:

and you can immediately work with ship.name and ship.data.

shipCoordList=gridClass.determineFullLocation(start,end)
gridClass.shipLocationDict[shipName]=shipCoordList

It seems odd to ask gridClass to work something out for you, and then immediately assign exactly that result back to an attribute on gridClass. It would make more sense if you could come up with a flow that the grid can modify it's own attributes appropriately. I think it might make sense for grid to have a method to place a single ship at a particular start location, and going in a particular direction - perhaps raising an exception if it would go past the end (possibly taking a flag to say that that's ok sometimes). It looks like you already have a method called grid.shipPlacement - why aren't you using it here?

Your coordinates here are strings. Instead, leave them as (x, y) tuples so you don't have to keep parsing them every time you want to do stuff with them.

Drop the word "determine" from various method names. It's just noise. We know that a function either does something or calculates something - ie, functions usually (although there are occasionally good reasons to have exceptions) follow the principle of command-query separation. So if its name is a noun phrase, we already know that it must calculate whatever that phrase refers to. You might consider making some of them into propertys so that you can use them as attributes.

StackExchange Code Review Q#103994, answer score: 11