This question is a follow-up question of this one.I've learnt a bit more Python since then. I recently came back to making battleships again, and I think I did better.  These are the changes I've made:Completely rewritten.  Multiple battleships.  Removed difficulty settings (as the way I was doing it before wouldn't apply to multiple battleships)  Separated UI and game logicThis is the code:A simple in-interpreter game of battleships.__version__ = 2.0.5from random import randintfrom math import sqrtdef getBoolFromInput(prompt, defaultReturn=None, yes=(), no=()):    Returns True / False / None from user input    inp = .join((char for char in input(prompt).lower() if char.isalpha()))#   inp is now all the alphabetic characters in the input    if inp in (yes, y, yh, definitely) or inp in yes:        return True    elif inp in (no, n, nope) or inp in no:        return False    return defaultReturndef getOrdinal(num):    Returns an ordinal string of the number that is passed. (e.g. 1 --> 1st, -32 --> -32nd, etc.    negative = False    if num < 0:        num *= -1        negative = True    unit = num % 10    if 4 <= num <= 20 or 4 <= unit <= 9 or unit == 0 or num == 0:        suffix = th    else:        suffix = [st, nd, rd][num % 10 - 1]    if negative:        num *= -1    return str(num) + suffixdef win(gameState):    print(Well done! You won battleships! That was your,          {} time today..format(getOrdinal(gameState.gamesWon)))    print(You have played {} games so far, losing {}..format(        gameState.gamesPlayed, gameState.gamesLost * -1))    return endText(gameState)def lose(gameState):    print(Oh no! You lost! That was your {} time today. Better luck.format(        getOrdinal(gameState.gamesLost * -1)), next time!)    print(You have played {} games so far, winning {}..format(        gameState.gamesPlayed, gameState.gamesWon))    return endText(gameState)def endText(gameState):    print(This give you {}% win and you have a {} game win streak..format(        gameState.gamesWon // gameState.gamesPlayed * 100,        gameState.winStreak))    while True:        inp = getBoolFromInput(Would you like to play again? ,                               None, (again), (notagain))        if inp is None:            print(Please type yes or no.)            continue        return inpclass Board:    def __init__(self, height, length):        self.height = height        self.length = length        self.items = []        for i in range(height):            self.items.append([])            for i2 in range(length):                self.items[i].append({Current mark: -,                                      Has battleship: False})    def __repr__(self):        Returns the board with   between every comlumn and \\n between every row.        return \n.join([ .join([self.items[h][l][Current mark] for h in                                    range(self.height)]) for l in range(                                        self.length)])    def updateMark(self, x, y):        if x > self.length or y > self.height or x < 1 or y < 1:            return OUT OF OCEAN        x -= 1        y -= 1#       -1 so a user's (1, 1) turns into an array index [0][0]        if self.items[y][x][Current mark] != -:            return ALREADY HIT        elif self.items[y][x][Has battleship]:            self.items[y][x][Current mark] = B            return HIT        self.items[y][x][Current mark] = X        return NO HIT    def revealBoard(self):        for y in range(self.height):            for x in range(self.length):                if self.items[y][x][Has battleship]:                    self.items[y][x][Current mark] = B                elif self.items[y][x][Current mark] == -:                    self.items[y][x][Current mark] = Oclass GameState:    def reset(self, turns, battleships):        self.turnsTotal = turns        self.turn = 0        self.turnsRemaining = turns        self.battleships = battleships        self.battleshipsHit = 0        self.battleshipsRemaining = battleships    def __init__(self, disableConfirmations):        self.disableConfirmations = disableConfirmations        self.gamesPlayed = 0        self.gamesWon = 0        self.gamesLost = 0        self.winStreak = 0    def nextTurn(self):        Increments turn and checks if the game has ended        self.turn += 1        self.turnsRemaining -= 1        if self.turnsRemaining == 0:            return self.gameFinished(False)        return True    def hitBattleship(self):        Increments amount of battleships hit and checks if all have been hit        self.battleshipsHit += 1        self.battleshipsRemaining -= 1        if self.battleshipsRemaining == 0:            return self.gameFinished(True)        return True    def gameFinished(self, result):        self.gamesPlayed += 1        if result:            if self.winStreak < 1:                self.winStreak = 0            self.winStreak += 1            self.gamesWon += 1            return win(self)        else:            if self.winStreak > -1:                self.winStreak = 0            self.winStreak -= 1            self.gamesLost -= 1            return lose(self)class Battleship:    def __init__(self, board, size):        while True:            east = True            self.x = randint(0, board.length - size)            self.y = randint(0, board.height - size)            self.direction = randint(0, 1)            valid = True            if self.direction == randint(0, 1):                self.direction = EAST                x = self.x                for i in range(size):                    if board.items[self.y][x][Has battleship]:                        valid = False                    x += 1            else:                self.direction = SOUTH                east = False                y = self.y                for i in range(size):                    if board.items[y][self.x][Has battleship]:                        valid = False                    y += 1            if valid:                if east:                    x = self.x                    for i in range(size):                        board.items[self.y][x][Has battleship] = True                        x += 1                else:                    y = self.y                    for i in range(size):                        board.items[y][self.x][Has battleship] = True                        y += 1                breakdef main():    try:        print(uHello! Welcome to battleships, version 2.0.Changelog:- Complete rewrite  - The game has multiple battleships now!  - No longer any difficulty setting (because of said multiple battleships)  - Now with \u221E% more classes!Now, let's get started!)    except UnicodeEncodeError:        print(Hello! Welcome to battleships, version 2.0.Changelog:- Complete rewrite  - The game has multiple battleships now!  - No longer any difficulty setting (because of said multiple battleships)  - Now with infinity% more classes!Now, let's get started!)    gameState = GameState(getBoolFromInput(Would you like to disable  +                                           confirmation messages? , False))#   Main game loop    while True:        s = tall        while True:            inp = input(How {} would you like the board? .format(s))            try:                inp = int(inp)            except ValueError:                print(Please type in a number!)                continue            if inp < 4:                print(Please type a value greater than 3!)            elif inp > 25:                if not getBoolFromInput(Do you really want a board dimensio +                                        n this big?, False):                    continue            if s == tall:                s = long                height = inp            else:                length = inp                if not gameState.disableConfirmations:                    a = getBoolFromInput(Are you alright with a  +                                         {} by {} board? .format(height,                                                                   length))                    if a is None:                        print(I'll take that as a \Yes\)                    elif a:                        print(OK! Let's continue!)                    else:                        s = tall                        print(Try again then.)                        continue                break        board = Board(height, length)        maxBattleships = int(0.5*sqrt(8*(height*length) + 1) - 0.5) - 2#       Finds which triangle number the area of the board is inbetween#       so you can't put more battleships than the board can take.        while True:            print(You can have a maximum of, maxBattleships, battleships,                  with your board size.)            inp = input(How many battleships would you like? )            try:                inp = int(inp)            except ValueError:                print(Please type a number!)                continue            if inp < 1:                print(Please enter a value greater than 0!)                continue            elif inp > maxBattleships:                print(Please type a value less than, maxBattleships)                continue            if not gameState.disableConfirmations:                a = getBoolFromInput(Are you alright with  + str(inp) +                                      battleships? )                if a is None:                    print('I\'ll take that as a Yes')                elif a:                    print(OK! Let's continue!)                else:                    print(Try again then.)                    continue            break        battleships = inp        while True:            inp = input(How many turns would you like? )            try:                inp = int(inp)            except ValueError:                print(Please type a number!)                continue            if inp < 1:                print(Please enter a value greater than 0!)                continue            if not gameState.disableConfirmations:                a = getBoolFromInput(Are you alright with  + str(inp) +                                      turns? )                if a is None:                    print(I'll take that as a \Yes\)                elif a:                    print(OK! Let's continue!)                else:                    print(Try again then.)                    continue            break        gameState.reset(inp, (battleships * (battleships + 1)) // 2)#                             This equation shows which triangle number#                             the battleship represents        allBattleships = {}        for i in range(battleships, 1, -1):            allBattleships[i] = Battleship(board, i)#           This makes the battleships increase in size, with their#           lengths like (1, 2, 3) or (1, 2, 3, 4) etc.        playing = True        while playing:            if gameState.turn == 0:                print(Let's play Battleships!\nTurn 1, {} turn{}.format(                    gameState.turnsRemaining, .join(                        [ if gameState.turnsRemaining == 1 else s                         for i in (1,)])), remaining.)            elif gameState.turnsRemaining == 1:                print(Last turn!)            else:                print(Turn {}, {} turns remaining..format(                    gameState.turn, gameState.turnsRemaining))            print(board)            s = Y            while True:                inp = input(s +  coordinate: )                try:                    inp = int(inp)                except ValueError:                    print(Please type a valid number!)                    continue                if s == Y:                    s = X                    x = inp                    continue                y = inp                if not gameState.disableConfirmations:                    inp = not getBoolFromInput(Are you sure that you want +                                               to aim for {}, {}? .format(                                                   x, y), True)                    if inp:                        print(Try again then.)                        s = Y                        continue                break            aim = board.updateMark(x, y)            if aim == OUT OF OCEAN:                print(That's not even in the ocean...)            elif aim == ALREADY HIT:                print(You already hit that one.)            elif aim == NO HIT:                print(You didn't hit anything...)            else:                print(Congratualtions, you hit part of a battleship!)                cont = gameState.hitBattleship()            if gameState.battleshipsRemaining != 0:                cont = gameState.nextTurn()            if gameState.turnsRemaining != 0 and \               gameState.battleshipsRemaining != 0:                continue            else:                board.revealBoard()                print(\n + str(board) + \nThese were the battleships!)            playing = False            if cont:                print(\n\nRestarting...\n\n)                continuing = True            else:                continuing = False        if continuing:            continue        print(Thanks for playing!)if __name__ == __main__:    main()I made it in a few hours (~3 to 4), so there are probably are some bugs; however, the game plays fine, and I can't find any bugs. I would like some feedback on 2 things:if the classes are necessary for readability or just confusingif my documentation using comments and docstrings is good enough.  Notes: I am using camelCase over underscore_case as I prefer it, and find it more readable.  I am repeating many similar while True: loops as they are so very slightly different, that I couldn't get around it.
1-player Battleship game - follow-up
python;game;python 3.x
Your imports should be ordered.You mentionedI am using camelCase over underscore_case as I prefer it, and find it more readable.but I'm still going to pick you up on it ;).get_bool_from_input should really have a separation of concerns; the logic that needs abstraction is going from the input to the boolean. As such, you should probably have bool_from_string and let the caller call input.In changing this, I noticed that you have inp = not getBoolFromInput(Are you sure that you want +                           to aim for {}, {}? .format(                               x, y), True)Which joins toAre you sure that you wantto aim for {}, {}? This kind of error is hidden by the somewhat haphazard formatting. Going with the change to bool_from_string, I would use something like:inp = not bool_from_string(    input(Are you sure that you want to aim for {}, {}? .format(x, y)),    True)Even better, split the logic into several source lines:question = Are you sure that you want to aim for {}, {}? .format(x, y)answer = not bool_from_string(input(question), True)...although immediately noting the result and not giving it a meaningful name is a surefire way to confuse readers on what the code is actually trying to do.Back to bool_from_string.I find the formatting a bit strange.The comment is formatted oddly as it should be flush with the text.There should be spacing after the doc comment.Comprehensions inside function calls don't need brackets.The doc comment is wrong and largely unhelpful.default_return should just be default.yes and no should probably be extra_yes and extra_no. These should probably have precedence over the internal options.The internal options should be sets.The comment could be removed if you just named inp better.The sets should probably be globals.YES_STRINGS = frozenset({yes, y, yh, definitely})NO_STRINGS = frozenset({no, n, nope})def bool_from_string(string, default=None, extra_yes=frozenset(), extra_no=frozenset()):        Tries to decide whether the input means yes or no.    Returns True if the input means yes, False if it means no    and default if it's unknown. If extra_yes or extra_no are passed in,    they must contain only lowercase alpha strings and will take    precidence over internal checks.        alpha_lower = .join(char for char in string if char.isalpha())    if alpha_lower in extra_yes:        return True    if alpha_lower in extra_no:        return False    if alpha_lower in YES_STRINGS:        return True    elif alpha_lower in NO_STRINGS:        return False    return defaultPersonally, though, it's better just to do:def bool_from_string(string, yes_strings=YES_STRINGS, no_string=NO_STRINGS, default=None):        Tries to decide whether the input means yes or no.    Returns True if the input means yes, False if it means no    and default if it's unknown. yes_strings and no_strings must    must contain only lowercase alpha strings.        alpha_lower = .join(char for char in string if char.isalpha())    if alpha_lower in YES_STRINGS:        return True    elif alpha_lower in NO_STRINGS:        return False    return defaultand callers can choose to call as bool_from_string(foo, YES_STRINGS | {foo}, NO_STRINGS | {bar}).I would also make default a keyword-only argument:def bool_from_string(string, yes_strings=YES_STRINGS, no_string=NO_STRINGS, *, default=None):These changes also highlight this problem:    inp = getBoolFromInput(Would you like to play again? ,                           None, (again), (notagain))Where (again) just means again and so the test inp in yes just checks that inp is a substring of again.getOrdinal again has some strange formatting. Just fixing that and cleaning up the docstring gives:def get_ordinal(num):        Returns an ordinal string of the number that is passed.    get_ordinal(0)  = 0th    get_ordinal(1)  = 1st    get_ordinal(2)  = 2nd    get_ordinal(-32) = -32nd        negative = False    if num < 0:        num *= -1        negative = True    unit = num % 10    if 4 <= num <= 20 or 4 <= unit <= 9 or unit == 0 or num == 0:        suffix = th    else:        suffix = [st, nd, rd][num % 10 - 1]    if negative:        num *= -1    return str(num) + suffixThe first block can be changed for:negative = num < 0num = abs(num)Your checkunit = num % 10if 4 <= num <= 20 or 4 <= unit <= 9 or unit == 0:doesn't need the final num == 0 as it implies unit == 0. However, it's easier just to invert the check:if unit in {1, 2, 3} and num not in {11, 12, 13}:    suffix = [st, nd, rd][num % 10 - 1]else:    suffix = thThis isn't enough, though, as get_ordinal(111) should return 111th (it currently gives 111st). You should instead doif unit in {1, 2, 3} and num % 100 not in {11, 12, 13}:    suffix = [st, nd, rd][num % 10 - 1]else:    suffix = thIt's strange you modify num and then reverse the change; just use a different variable instead. Since the change is so simple, you can just stick with:def get_ordinal(num):        Returns an ordinal string of the number that is passed.    get_ordinal(0)  = 0th    get_ordinal(1)  = 1st    get_ordinal(2)  = 2nd    get_ordinal(-32) = -32nd        mag = abs(num)    if mag % 10 in {1, 2, 3} and mag % 100 not in {11, 12, 13}:        suffix = [st, nd, rd][mag % 10 - 1]    else:        suffix = th    return str(num) + suffixI would also suggest replacingsuffix = [st, nd, rd][mag % 10 - 1]withsuffix = {1: st, 2: nd, 3: rd}[mag % 10]In fact, you could even write:# 11th, 12th, 13thif mag % 100 in {11, 12, 13}:    suffix = thelse:    suffix = {1: st, 2: nd, 3: rd}.get(mag % 10, th)win is slightly misnamed; it only tells people that they have won and asks if they want to play again. This would be better named alert_won_and_retry. However, I would probably not conflate the two options and just have alert_won, leaving the caller to run end_text. It so happens that this would simplify code, too.The formatting here is also haphazard. Line wrapping should be logical, not just whereever it fits.print(Well done! You won battleships! That was your,      {} time today..format(getOrdinal(gameState.gamesWon)))print(You have played {} games so far, losing {}..format(    gameState.gamesPlayed, gameState.gamesLost * -1))This also hides the same line-concatenation bug as before.My personal style would useprint(Well done! You won battleships! That was your {} time today.format(    get_ordinal(game_state.games_won)))print(You have played {} games so far, losing {}..format(    game_state.games_played,    game_state.games_lost * -1))What matters isn't that your style is the same as mine or that it sticks strictly to line length guidelines but that it makes bugs look wrong. Whether this means you write things on loads of different lines:number_wins_alert = Well done! You won battleships! That was your {} time todaynumber_wins_alert = number_wins_alert.format(get_ordinal(game_state.games_won))print(number_wins_alert)number_games_alert = You have played {} games so far, losing {}.number_games_alert = number_games_alert.format(game_state.games_played, game_state.games_lost * -1)print(number_games_alert)or do Lisp-style expression embebedding, the aim is to make the structure and logic coincide.Exactly the same arguments apply to lost.end_text is really misnamed; it should be something like alert_finish_and_ask_replay. This might suggest again that you split it up into alert_finish and ask_replay.I have alert_finish doing:def alert_finish(game_state):    print(This give you {}% win and you have a {} game win streak..format(        game_state.games_won // game_state.games_played * 100,        game_state.win_streak    ))This is buggy; game_state.games_won // game_state.games_played does integer division and then * 100 will always give you 100 or 0. You should have done int(game_state.games_won / game_state.games_played * 100)or, better, just used the {:.0%} format specifier. The .0 says there should be 0 fractional digits and the % displays the number as a percentage.print(This give you {}% win and you have a {:.0%} game win streak..format(    game_state.games_won / game_state.games_played,    game_state.win_streak))All of these GameStates are getting to me: it's a class so of course it's state. You should just call it game.Looking at Board.__init__, we have the generation of a grid. This is simpler written as a comprehension:cell = {Current mark: -, Has battleship: False}self.items = [[cell.copy() for _ in range(length)] for _ in range(height)]I would be really tempted to change the grid cells to objects:class Cell:    def __init__(self, current_mark=-, has_battleship=False):        self.current_mark = current_mark        self.has_battleship = has_battleshipclass Board:    def __init__(self, height, length):        self.height = height        self.length = length        self.items = [[Cell() for _ in range(length)] for _ in range(height)]Board.__repr__'s docstring should be reformatted again. Either likeReturns the board with   between every column and \\n between every row.or likeReturns the board with   between every column and\\n between every row.Further, the lines should be split up from their current squished self:I would also use words to describe newlines.Transposing can be done with zip(*self.items), givingdef __repr__(self):        Returns the board with spaces between comlumns and rows on separate lines.        transposed = zip(*self.items)    return \n.join( .join(cell.current_mark for cell row) for row in transposed)updateMark should really not be returning strings, but with this organization I don't see anything much easier than an enum.class AttackSuccess(Enum):    hit = 1    miss = 2    already_hit = 3    out_of_bounds = 4At point of use you should doif aim = AttackSuccess.hit:    print(Congratualtions, you hit part of a battleship!)    cont = game.hit_battleship()elif aim == AttackSuccess.miss:    print(You didn't hit anything...)elif aim == AttackSuccess.out_of_bounds:    print(That's not even in the ocean...)elif aim == AttackSuccess.already_hit:    print(You already hit that one.)else:    raise NotImplementedErrorwhich will trigger an error if the enum gets extended and the point of use isn't, not just break silently.Again, the comment should be flush with the text.if x > self.length or y > self.height or x < 1 or y < 1:        return AttackSuccess.out_of_boundswould be better asif not (1 <= x <= self.length and 1 <= y <= self.height):        return AttackSuccess.out_of_boundsPersonally, though, I would do x -= 1; y -= 1 before calling the function: internal functions shouldn't be exposed to user-facing details like 1-based indexing.No need for elif if you've already returned.The name could be more descriptive; perhaps attack_cellThis gives:def attack_cell(self, x, y):    if not (0 <= x < self.length and 0 <= y < self.height):        return AttackSuccess.out_of_bounds    if self.items[y][x].current_mark != -:        return AttackSuccess.already_hit    if self.items[y][x].has_battleship:        self.items[y][x].current_mark = B        return AttackSuccess.hit    self.items[y][x].current_mark = X    return AttackSuccess.missrevealBoard is largely OK.GameState should have its __init__ function first. Its init function doesn't initialize all of its values; you have to then call reset to get that running. This suggests you have a bad separation of concerns; Game should be split into two classes where one handles the overall statistics and another handles the game itself. I would go with GameStatistics and BattleshipsGame.With this, gameFinished should be split into three parts; alert_finish and and finish_game which are part of GameStatistics. The global alert_finish function can be merged into the local alert_finish method and the alert_won and alert_lost functions should become methods.ask_replay would be called from the code that handles controlling the game (currently main) as neither class has any business dealing with it. next_turn and hit_battleship should return whether the game has ended ignoring whether the user wants to replay.turnsTotal and batteships and aren't used, so remove them.GameStatistics.finish_game would bedef finish_game(self, result):    self.games_played += 1    if result:        if self.win_streak < 1:            self.win_streak = 0        self.win_streak += 1        self.games_won += 1    else:        if self.win_streak > -1:            self.win_streak = 0        self.win_streak -= 1        self.games_lost -= 1You write games_lost -= 1 but should really write games_lost += 1; you end up multiplying the result by -1 whenever you use it anyway! You can also extract the zeroing logic:def finish_game(self, result):    self.games_played += 1    if (self.win_streak > 0) != result:        self.win_streak = 0    if result:        self.win_streak += 1        self.games_won += 1    else:        self.win_streak -= 1        self.games_lost += 1It might be better to stick with the origingal logic for zeroing if this doesn't doesn't seem self explanatory enough, but it's the same deal as the old cmp function or Java's compareTo.disableConfirmations would be better named as a positive; confirmations_enabled. It should also be removed from the GameStatistics class because it's neither used internally nor really benefits from encapsulation.The Battleship class is a behemoth with only one method; __init__. This seems completely the wrong way to do it, especially because __init__ shouldn't really be mutating things. If you want battleships to be made inside boards, add a method to the board class. In fact, you write:allBattleships = {}for i in range(battleships, 1, -1):    allBattleships[i] = Battleship(board, i)but never touch allBattleships again! This implied that a class is really the wrong abstraction in some sense; you're not using Battleship to store state but to stick itself onto the board somehow. Get rid of the class entirely.In the constructor you use self.direction for two purposes. One is to hold a random boolean you use once and another is to hold an unused string. The first can be replaced with a random.choice([True, False]) at point of use and the second can be removed entirely.This can be further simplified by just writing east = random.choice([True, False]) and not mutating it further.Your loops can be replaced with ranges using the start and stop parameters.This all gives:def add_battleship(self, size):    while True:        east = random.choice([True, False])        start_x = random.randint(0, self.length - size)        start_y = random.randint(0, self.height - size)        valid = True        if east:            for x in range(start_x, start_x+size):                if self.items[start_y][x].has_battleship:                    valid = False        else:            for y in range(start_y, start_y+size):                if self.items[y][start_x].has_battleship:                    valid = False        if valid:            break    if east:        for x in range(start_x, start_x+size):            self.items[start_y][x].has_battleship = True    else:        for y in range(start_y, start_y+size):            self.items[y][start_x].has_battleship = TrueIt makes sense to view these as rectangles to remove the two cases:def add_battleship(self, size):    while True:        start_x = random.randint(0, self.length - size)        start_y = random.randint(0, self.height - size)        valid = True        if random.choice([True, False]):            width, height = size, 1        else:            width, height = 1, size        for x in range(start_x, start_x+width):            for y in range(start_y, start_y+height):                if self.items[y][x].has_battleship:                    valid = False        if valid:            break    for x in range(start_x, start_x+width):        for y in range(start_y, start_y+height):            self.items[y][x].has_battleship = TrueYou could also use all here:def add_battleship(self, size):    while True:        start_x = random.randint(0, self.length - size)        start_y = random.randint(0, self.height - size)        if random.choice([True, False]):            width, height = size, 1        else:            width, height = 1, size        xs = range(start_x, start_x+width)        ys = range(start_y, start_y+height)        if not any(self.items[y][x].has_battleship for x in xs for y in ys):            break    for x in xs:        for y in ys:            self.items[y][x].has_battleship = Truemain is the real beast here, but I'm going to wait before tackling that.I will post the updated code in another answer because I have run out of characters to add it to this one.