3
\$\begingroup\$

The following is some code for a Battleship game I'm working on. The plan is to create a multiplayer game using Xamarin Forms. This is just the model code, i.e. what the GUI will be interacting with when playing the game. I'd like some advice on conventions and any other code optimizations that can be found. Any help is greatly appreciated!

ISerializable.cs

namespace Battleship
{
    public interface ISerializable
    {
        string Serialize();
    }
}

GridSquare.cs

using System;

namespace Battleship
{
    public enum SquareType
    {
        Foggy,
        Water,
        Undamaged,
        Damaged,
        Sunk
    }

    public class GridSquare : ISerializable
    {
        public int Row { get; private set; }
        public int Column { get; private set; }
        public SquareType Type { get; set; }
        public int ShipIndex { get; set; }

        private readonly bool _charted;

        public GridSquare(int row, int column, bool charted)
        {
            _charted = charted;
            Row = row;
            Column = column;
            Type = charted ? SquareType.Water : SquareType.Foggy;
            ShipIndex = -1;
        }

        public string Serialize()
        {
            return GetEncodedNumber(Row) + ":" + GetEncodedNumber(Column) + ":" + _charted.ToString() + ":" + GetEncodedNumber(ShipIndex) + ":" + Type.ToString();
        }

        private string GetEncodedNumber(int n)
        {
            return n % 10 == 0 || n < 0 ? n.ToString() : n.ToString().PadLeft('0');
        }

        public static GridSquare Deserialize(string data)
        {
            string[] msg = data.Split(':');

            return new GridSquare(Convert.ToInt32(msg[0]), Convert.ToInt32(msg[1]), Convert.ToBoolean(msg[2]))
            {
                ShipIndex = Convert.ToInt32(msg[3]),
                Type = (SquareType)Enum.Parse(typeof(SquareType), msg[5])
            };
        }
    }
}

Grid.cs

using System;
using System.Collections.Generic;
using System.Linq;

namespace Battleship
{
    public class Grid : ISerializable
    {
        public List<List<GridSquare>> Squares { get; private set; }

        private readonly int _gridSize;

        public Grid(int gridSize)
        {
            _gridSize = gridSize;
            Squares = new List<List<GridSquare>>(gridSize);
        }

        public int GetSize()
        {
            return _gridSize;
        }

        public string Serialize()
        {
            string data = _gridSize + ":";
            List<string> info = new List<string>(_gridSize);

            for (int i = 0; i < _gridSize; ++i)
            {
                for (int j = 0; j < _gridSize; ++j)
                {
                    info.Add(Squares[i][j].Serialize());
                }
            }

            return data + string.Join(",", info.ToArray());
        }

        public static Grid Deserialize(string data)
        {
            string[] msg = data.Split(':');
            Grid grid = new Grid(Convert.ToInt32(msg[0]));

            foreach (string squareData in msg[1].Split(',').ToList())
            {
                // technically this is order insensitive as well
                GridSquare square = GridSquare.Deserialize(squareData);
                grid.Squares[square.Row][square.Column] = square;
            }

            return grid;
        }
    }
}

Ship.cs

using System;

namespace Battleship
{
    // the classic rules (patrol boats aren't fit for warfare)
    public enum ShipType
    {
        Carrier = 5,
        Battleship = 4,
        Cruiser = 3,
        Submarine = 3,
        Destroyer = 2
    }

    public enum Orientation
    {
        South = 0,
        East = 1,
        North = 2,
        West = 3
    }

    static class OrientationMethods
    {
        public static Orientation FromIndex(this int n)
        {
            switch (n)
            {
                case 1:
                    return Orientation.East;
                case 2:
                    return Orientation.North;
                case 3:
                    return Orientation.West;
                default:
                    return Orientation.South;
            }
        }
    }

    public class Ship : ISerializable
    {
        public int Health { get; set; }
        public Orientation Orientation { get; set; }

        private readonly ShipType _type;

        public Ship(ShipType type)
        {
            _type = type;
            Repair();
        }

        public int Length()
        {
            return (int)_type;
        }

        public void Repair()
        {
            Health = Length();
        }

        public bool Sunk()
        {
            return Health == 0;
        }

        public void Hit()
        {
            if (Health > 0)
            {
                --Health;
            }
        }

        public string Serialize()
        {
            return _type.ToString() + ":" + Health;
        }

        public static Ship Deserialize(string data)
        {
            string[] msg = data.Split(':');
            return new Ship((ShipType)Enum.Parse(typeof(ShipType), msg[0]))
            {
                Health = Convert.ToInt32(msg[1])
            };
        }
    }
}

Player.cs

using System;
using System.Collections.Generic;
using System.Linq;

namespace Battleship
{
    public class Player : ISerializable
    {
        public Grid Grid { get; set; }
        public List<Ship> Ships { get; set; }

        protected static Random rand = new Random();

        public Player(int gridSize)
        {
            Grid = new Grid(gridSize);
            Ships = new List<Ship>();

            foreach (ShipType type in Enum.GetValues(typeof(ShipType)))
            {
                Ships.Add(new Ship(type));
            }

            PlaceShips();
        }

        private void PlaceShips()
        {
            for (int shipIndex = 0; shipIndex < Ships.Count; ++shipIndex)
            {
                int x = rand.Next(Grid.GetSize());
                int y = rand.Next(Grid.GetSize());
                GridSquare sea = Grid.Squares[x][y];
                Ship ship = Ships[shipIndex];
                List<Orientation> validOrientations = new List<Orientation>();

                // calculate valid orientations
                for (int i = 0; i < 4; ++i)
                {
                    bool done = false;

                    try
                    {
                        for (int j = 1; j < ship.Length() && !done; ++j)
                        {
                            Orientation o = OrientationMethods.FromIndex(i);

                            switch (o)
                            {
                                case Orientation.South:
                                    if (Grid.Squares[x][y - j].ShipIndex != -1)
                                    {
                                        done = true;
                                    }
                                    break;
                                case Orientation.East:
                                    if (Grid.Squares[x - j][y].ShipIndex != -1)
                                    {
                                        done = true;
                                    }
                                    break;
                                case Orientation.North:
                                    if (Grid.Squares[x][y + j].ShipIndex != -1)
                                    {
                                        done = true;
                                    }
                                    break;
                                case Orientation.West:
                                    if (Grid.Squares[x + j][y].ShipIndex != -1)
                                    {
                                        done = true;
                                    }
                                    break;
                            }

                            if (j == Grid.GetSize() - 1)
                            {
                                validOrientations.Add(o);
                            }
                        }
                    }
                    catch (Exception) { }
                }

                if (!validOrientations.Any())
                {
                    throw new Exception("The current grid cannot fit all of the ships!");
                }

                // set the origin metadata
                sea.Type = SquareType.Undamaged;
                sea.ShipIndex = shipIndex;
                ship.Orientation = validOrientations[rand.Next(validOrientations.Count)];

                // pick an orientation at random and layout the ship
                for (int i = 1; i < ship.Length(); ++i)
                {
                    switch (ship.Orientation)
                    {
                        case Orientation.South:
                            Grid.Squares[x][y - i].ShipIndex = shipIndex;
                            break;
                        case Orientation.East:
                            Grid.Squares[x - i][y].ShipIndex = shipIndex;
                            break;
                        case Orientation.North:
                            Grid.Squares[x][y + i].ShipIndex = shipIndex;
                            break;
                        case Orientation.West:
                            Grid.Squares[x + i][y].ShipIndex = shipIndex;
                            break;
                    }
                }
            }
        }

        public void Attack(int x, int y)
        {
            GridSquare sea = Grid.Squares[x][y];

            switch (sea.Type)
            {
                case SquareType.Foggy: // miss
                    sea.Type = SquareType.Water;
                    break;
                case SquareType.Undamaged:
                    Ship ship = Ships[sea.ShipIndex];

                    ship.Hit();

                    if (ship.Sunk())
                    {
                        sea.Type = SquareType.Sunk;

                        for (int i = 1; i < ship.Length(); ++i)
                        {
                            switch (ship.Orientation)
                            {
                                case Orientation.South:
                                    Grid.Squares[x][y - i].Type = SquareType.Sunk;
                                    break;
                                case Orientation.East:
                                    Grid.Squares[x - i][y].Type = SquareType.Sunk;
                                    break;
                                case Orientation.North:
                                    Grid.Squares[x][y + i].Type = SquareType.Sunk;
                                    break;
                                case Orientation.West:
                                    Grid.Squares[x + i][y].Type = SquareType.Sunk;
                                    break;
                            }
                        }
                    }
                    else
                    {
                        sea.Type = SquareType.Damaged;
                    }

                    break;
                default:
                    // unallowed operation that should be handled client-side
                    break;
            }
        }

        public bool GameOver()
        {
            return Ships.All(ship => ship.Sunk());
        }

        public string Serialize()
        {
            return Grid.Serialize() + "|" + SerializeShips();
        }

        private string SerializeShips()
        {
            List<string> info = new List<string>();

            foreach (Ship ship in Ships)
            {
                info.Add(ship.Serialize());
            }

            return string.Join(",", info.ToArray());
        }

        public static Player Deserialize(string data)
        {
            string[] info = data.Split('|');
            Grid grid = Grid.Deserialize(info[0]);

            return new Player(grid.GetSize())
            {
                Grid = grid,
                Ships = DeserializeShips(info[1])
            };
        }

        private static List<Ship> DeserializeShips(string data)
        {
            string[] info = data.Split(',');
            List<Ship> ships = new List<Ship>();

            foreach (string s in info)
            {
                ships.Add(Ship.Deserialize(s));
            }

            return ships;
        }
    }
}
\$\endgroup\$
0
\$\begingroup\$

Global

Why don't you use the usual serialization stuff instead of creating you Interface (which by the way has the same name than System.Runtime.Serialization.ISerializable) and doing some kind of weird tinkering to serialize and deserialize?

Grid.cs

1) foreach (string squareData in msg[1].Split(',').ToList()) : you don't need to convert the result of .Split() to a List as .Split() returns an array and foreach works on arrays too.

2)

private readonly int _gridSize;
public int GetSize()
{
    return _gridSize;
}

I don't realy get the point of GetSize() here, you could just change _gridSize to public and remove GetSize().

3) You shouldn't use a List<List<GridSquare>> but instead you should use a GridSquare[][] it will at least help you being sure everything has been initialized.

4) Same kind of useless conversion than in 1) : return data + string.Join(",", info.ToArray()); you don't need to do .ToArray() here.

Ship.cs

1) Length() should be renamed to GetLength().

2) You miss something like the ship's top left corner's position.

GridSquare.cs

1) Row and column are useless as we already know the current position because we must access the tile thanks to two index because the real board is Grid.Squares which is a GridSquare[][].

2) ShipIndex doesn't look realy useful too, what was it meant to do?

3) Why is _charted readonly? It's meant to change when the opponent fires on it.

Player.cs

1) In the PlaceShips() method you are doing catch (Exception) { } which should i think never be done.

2) In the Attack() method you have

switch(sea.Type)
{
    ...
    default:
        //unallowed operation that should be handled client-side
        break;
}

Well if it's not allowed then I think you should throw an exception.

I didn't test anything, assuming that you can do tests on your own, don't hesitate to reply through comments if you have any question.

\$\endgroup\$
  • \$\begingroup\$ I'm trying to serialize and de-serialize the data to allow game saving in Xamarin Forms through Application.Current.Properties. If you can think of a better way to convert them to a string object or integer data type I'd love to hear it! Your other notes on semantics I agree with. \$\endgroup\$ – T145 Oct 4 '18 at 20:40
  • \$\begingroup\$ well i'd recommend using XML Serialization (System.Xml.Serialization) you can find examples on MSDN or whatever or just understand how it works with descriptions if you are using visual studio \$\endgroup\$ – nalka Oct 4 '18 at 20:55
  • \$\begingroup\$ Also for PlaceShips it might leave the array, and I don't want the whole program to crash. Instead, I just want to know that direction is invalid b/c it leaves the array and leave it at that. \$\endgroup\$ – T145 Oct 4 '18 at 21:01
  • \$\begingroup\$ yes but i think you catch should target only System.IndexOutOfRangeException if i understood correctly how the algo is meant to work \$\endgroup\$ – nalka Oct 4 '18 at 21:08
  • \$\begingroup\$ oh also, why did you do an extension method? It could just have been a normal one \$\endgroup\$ – nalka Oct 4 '18 at 21:30

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.