2

when I run this program, sometimes I receive an error.This error however is not possible as I am using an 8x8 grid and I limit the inputs so that they can only be numbers from 0-7, to obey the fact that list indexes start at 0.

The user must input coordinates (1-8),(A-H) and the program will check to see if those coordinates are correct, by systematically going through the CompShips list and repeatedly comparing those coordinates to ones given by the user. If the cords match, then a message will appear and a "Z" will change to an "X" on those coordinates, indicating a HIT. If the guess does not match, a "Z" will change to an "M" on those coordinates indicating a MISS.

CompShips=[[1,0],[1,1],[2,2],[2,3],[2,4],[3,0],[3,1],[3,2],[5,4],[5,5],[5,6],[5,7],[1,7],[2,7],[3,7],[4,7],[5,7]] 
FRow1=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow2=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow3=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow4=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow5=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow6=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow7=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow8=["Z","Z","Z","Z","Z","Z","Z","Z",]
def PrintFireBoard():
    print(Index)
    print(FRow1)
    print(FRow2)
    print(FRow3)
    print(FRow4)
    print(FRow5)
    print(FRow6)
    print(FRow7)
    print(FRow8)
FireBoard=[FRow1,FRow2,FRow3,FRow4,FRow5,FRow6,FRow7,FRow8]
while len(CompShips) !=0 or CompSuccess==17:
    FireRow=input("Please Choose The Row That You Wish To Fire Upon (1-8) ")
    FireIndex=input("Please Choose The Column That You Wish To Fire Upon (A-H) ")
    #As Lists start at 0
    FireRow=int(FireRow)-1
    if FireIndex==("A"):
        FireIndex=0
    elif FireIndex==("B"):
        FireIndex=1
    elif FireIndex==("C"):
        FireIndex=2
    elif FireIndex==("D"):
         FireIndex=3
    elif FireIndex==("E"):
        FireIndex=4
    elif FireIndex==("F"):
        FireIndex=5
    elif FireIndex==("G"):
        FireIndex=6
    elif FireIndex==("H"):
        FireIndex=7
Guess=[FireRow,FireIndex]
#Check To See If Correct
UserSuccess=0
for i in CompShips:
    if Guess==i:
        CompShips.remove(Guess)
        UserSuccess=1
    else:
        pass
if UserSuccess==1:
    print("HIT")
    print(FireRow)
    print(FireIndex)
    FireBoard[[FireRow][FireIndex]]=("H")
    PrintFireBoard()
else:
    print("MISS")
    print(FireRow)
    print(FireIndex)
    FireBoard[[FireRow][FireIndex]]=("M")
    PrintFireBoard()

I receive the error:

IndexError: string index out of range
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Chocoblow
  • 23
  • 2
  • 1
    Does it tell you on which line `IndexError` occurred? – kgf3JfUtW Dec 20 '17 at 17:15
  • 2
    Just a small comment, instead of building the board like that, you can use this nifty trick to expand items into a list. `["Z"] * 8` will produce `['Z', 'Z', 'Z', 'Z', 'Z', 'Z', 'Z', 'Z']` so `[["Z"]*8]*8` will produce an 8x8 list of list with "Z" as the elements. – Axel Persinger Dec 20 '17 at 17:20
  • 2
    @AxelPersinger That will create 8 copies of the outer array, so if you modify one, they're all changed. You need to do this: `[['Z' for i in range(8)] for j in range(8)]` – c2huc2hu Dec 20 '17 at 17:48
  • @user3080953, that's correct, I totally forgot about that! Your answer is the correct way to do it. – Axel Persinger Dec 20 '17 at 17:51
  • 1
    @AxelPersinger: `["Z"]*8` is ok because strings are immutable, and any operation on them actually replaces the object; but `[["Z"]*8]*8` is a big no - you are going to get a list containing 8 references to the same sublist, so if you do - say `FireBoard[0][0]="A"` you'll get an `A` in the first column of each row - because all rows refer to the same list. The correct way to go is `[["Z"]*8 for _ in range(8)]`. – Matteo Italia Dec 20 '17 at 17:52
  • Uh I was preceded. – Matteo Italia Dec 20 '17 at 17:52

3 Answers3

1

Looks like these two lines

FireBoard[[FireRow][FireIndex]]=("H")
FireBoard[[FireRow][FireIndex]]=("M")

should be

FireBoard[FireRow][FireIndex]="H"
FireBoard[FireRow][FireIndex]="M"

Explanation: In your old code, FireBoard[[FireRow][FireIndex]]=("H")

[FireRow][FireIndex] means, given a list [FireRow] (which contains just one element), get the FireIndex-th element. This is not what you're trying to do.

For example [3][0] returns 3, and [3][1] gives IndexError.

Take a look at How to define a two-dimensional array in Python

Also note that ("H") is the same as the string "H". There is no need to add parentheses.

kgf3JfUtW
  • 13,702
  • 10
  • 57
  • 80
0

The indentation in your question was off. I think that all the code from

 Guess=[FireRow,FireIndex]

until the end should be preceded by 4 spaces.

I've removed print(Index) since it was not defined.

To access FireBoard use:

FireBoard[FireRow][FireIndex]

Instead of

FireBoard[[FireRow][FireIndex]]

This should be working

CompShips=[[1,0],[1,1],[2,2],[2,3],[2,4],[3,0],[3,1],[3,2],[5,4],

[5,5],[5,6],[5,7],[1,7],[2,7],[3,7],[4,7],[5,7]] 
FRow1=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow2=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow3=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow4=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow5=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow6=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow7=["Z","Z","Z","Z","Z","Z","Z","Z",]
FRow8=["Z","Z","Z","Z","Z","Z","Z","Z",]
def PrintFireBoard():
    print(FRow1)
    print(FRow2)
    print(FRow3)
    print(FRow4)
    print(FRow5)
    print(FRow6)
    print(FRow7)
    print(FRow8)
FireBoard=[FRow1,FRow2,FRow3,FRow4,FRow5,FRow6,FRow7,FRow8]
while len(CompShips) !=0 or CompSuccess==17:
    FireRow=input("Please Choose The Row That You Wish To Fire Upon (1-8) ")
    FireIndex=input("Please Choose The Column That You Wish To Fire Upon (A-H) ")
    #As Lists start at 0
    FireRow=int(FireRow)-1
    if FireIndex==("A"):
        FireIndex=0
    elif FireIndex==("B"):
        FireIndex=1
    elif FireIndex==("C"):
        FireIndex=2
    elif FireIndex==("D"):
         FireIndex=3
    elif FireIndex==("E"):
        FireIndex=4
    elif FireIndex==("F"):
        FireIndex=5
    elif FireIndex==("G"):
        FireIndex=6
    elif FireIndex==("H"):
        FireIndex=7
    Guess=[FireRow,FireIndex]
    #Check To See If Correct
    UserSuccess=0
    for i in CompShips:
        if Guess==i:
            CompShips.remove(Guess)
            UserSuccess=1
        else:
            pass
    if UserSuccess==1:
        print("HIT")
        print(FireRow)
        print(FireIndex)
        FireBoard[FireRow][FireIndex]=("H")
        PrintFireBoard()
    else:
        print("MISS")
        print(FireRow)
        print(FireIndex)
        FireBoard[FireRow][FireIndex]=("M")
        PrintFireBoard()
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Diego Sacconi
  • 120
  • 1
  • 8
0

Here is a much cleaner code!

CompShips=[[1,0],[1,1],[2,2],[2,3],
           [2,4],[3,0],[3,1],[3,2],
           [5,4],[5,5],[5,6],[5,7],
           [1,7],[2,7],[3,7],[4,7],
           [5,7]] 

FRow=[["Z"]*8]*8   #1 More Pythonic


def PrintFireBoard():
    #print(Index)
    for i in range(0,8):
        print(FRow[i])

FireBoard=FRow[:]  #NOTE THIS ONE!!!

mydict = {}
for i,key in enumerate(["A","B","C","D","E","F","G","H"]): #2 More Pythonic
    mydict[key] = i


while len(CompShips) !=0 or CompSuccess==17:
    FireRow=input("Please Choose The Row That You Wish To Fire Upon (1-8) ")
    FireIndex=input("Please Choose The Column That You Wish To Fire Upon (A-H) ")

    FireRow=int(FireRow)-1
    FireIndex = mydict[FireIndex]
    Guess=[FireRow,FireIndex]
    print(Guess)

    UserSuccess=0
    for i in CompShips:
        if Guess==i:
            CompShips.remove(Guess)
            UserSuccess=1
        else:
            pass
    if UserSuccess==1:
        print("HIT")
        print(FireRow,FireIndex)
        FireBoard[FireRow][FireIndex]="H" #3 your problem here
        PrintFireBoard()
    else:
        print("MISS")
        print(FireRow,FireIndex)
        FireBoard[FireRow][FireIndex]="M"
        PrintFireBoard()

1) As explained in the comments that's just a more nicer way to create a list of lists!. Remember DRY principle! Do Not Repeat yourself!

2) Instead of having all that if else to convert the 'A' to 0. You can use a dictionary lookup instead!

3) Your problem seems to be here! correct this to FireBoard[FireRow][FireIndex]="H"

PS: NOTE THIS ONE!!!: I'm not just making FireBoard as an alias to FRow! I'm copying it into a FireBoard as a new list! There's a subtle difference read about it here. I'm doing this incase you don't want your original FRow list to be modified!

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
void
  • 2,571
  • 2
  • 20
  • 35