6
\$\begingroup\$

I've created Snake in Console. I've tried to make my code as good as I possibly could. (Code on github: https://github.com/bartex-bartex/SnakeCSharp)

Question

Is my coding style good enough, so I can move to creating another project while not repeating some blunders I've made here?

Program.cs
using SnakeGame;

Game game = new Game(40, 30);
game.Start();
Game.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace SnakeGame
{
    public class Game
    {
        public Board Board { get; set; }
        public Snake Snake { get; set; }
        public Apple Apple { get; set; }
        public Score Score { get; set; }

        private const int scoreTextHeight = 1;
        private const int FPS = 16; 


        public Game(int width, int height)
        {
            Board = new Board(width, height);
            Snake = new Snake(width, height);
            Apple = new Apple(width, height, Snake.Position);
            Score = new Score();

            Console.CursorVisible = false;
            Console.SetWindowSize(width, height + scoreTextHeight);
        }

        public void Start()
        {
            Point previousSnakeTail = new Point(Snake.GetSnakeTail().x, Snake.GetSnakeTail().y);
            while (true)
            {
                Board.Draw(Snake.Position, previousSnakeTail, Apple.Position);
                previousSnakeTail = Snake.GetSnakeTail();

                // Snake Movement
                Direction direction = KeyboardManager.GetDirection();
                bool isBodyCollision = Snake.Move(direction, Board.Width, Board.Height);
                if (isBodyCollision) break;
                

                // Handle apple eat
                if (Apple.CheckIfEaten(Snake.GetSnakeHead()))
                {
                    Snake.IncreaseLength();
                    Score.IncreasePoints();

                    Apple = new Apple(Board.Width, Board.Height, Snake.Position);
                }

                Score.PrintScore(Board.Height);
                Thread.Sleep(1000 / FPS);
            }

            Messages.GameOverMessage(Board.Width, Board.Height, Score.GetPoints());
            Console.ReadLine();
        }
    }
}
Snake.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Security;
using System.Text;
using System.Threading.Tasks;

namespace SnakeGame
{
    public struct Point
    {
        public int x;
        public int y;

        public Point(int x, int y)
        {
            this.x = x;
            this.y = y;
        }

        public static bool operator== (Point p1, Point p2)
        {
            if(p1.x == p2.x && p1.y == p2.y)
            {
                return true;
            }
            return false;
        }

        public static bool operator !=(Point p1, Point p2)
        {
            if (p1.x != p2.x || p1.y != p2.y)
            {
                return true;
            }
            return false;
        }
    }

    public enum Direction { previousDirection, up, right, down, left }


    public class Snake
    {
        public List<Point> Position { get; private set; }
        private Direction currentDirection;

        public Snake(int mapWidth, int mapHeight)
        {
            Position = new List<Point>();

            //Spawn snake head in the middle
            Position.Add(new Point(mapWidth / 2, mapHeight / 2));

            //Initiate movement
            currentDirection = Direction.up;
        }

        public bool Move(Direction newDirection, int mapWidth, int mapHeight)
        {
            currentDirection = UpdateCurrentDirection(newDirection);

            switch (currentDirection)
            {
                case Direction.up:
                    UpdateSnakePositionList(Direction.up, mapWidth, mapHeight, 
                        new Point(Position[0].x, mapHeight - 2), new Point(Position[0].x, Position[0].y - 1));
                    break;
                case Direction.right:
                    UpdateSnakePositionList(Direction.right, mapWidth, mapHeight,
                        new Point(1, Position[0].y), new Point(Position[0].x + 1, Position[0].y));
                    break;
                case Direction.down:
                    UpdateSnakePositionList(Direction.down, mapWidth, mapHeight,
                        new Point(Position[0].x, 1), new Point(Position[0].x, Position[0].y + 1));
                    break;
                case Direction.left:
                    UpdateSnakePositionList(Direction.left, mapWidth, mapHeight,
                        new Point(mapWidth - 2, Position[0].y), new Point(Position[0].x - 1, Position[0].y));
                    break;
                default:
                    break;
            }
            return CheckIfCollisionWithTail();
        }

        private Direction UpdateCurrentDirection(Direction newDirection)
        {
            if (newDirection != Direction.previousDirection)
            {
                if (Position.Count == 1)
                { 
                    return newDirection;
                }
                if (CheckIfOppositeDirection(newDirection) == true)
                {
                    return currentDirection;
                }
                else
                {
                    return newDirection;
                }
            }
            else
            {
                return currentDirection;
            }
        }

        private bool CheckIfOppositeDirection(Direction newDirection)
        {
            if (currentDirection == Direction.left && newDirection == Direction.right ||
                currentDirection == Direction.right && newDirection == Direction.left ||
                currentDirection == Direction.up && newDirection == Direction.down ||
                currentDirection == Direction.down && newDirection == Direction.up)
            {
                return true;
            }
            return false;
        }

        private bool CheckIfCollisionWithBoundary(Direction direction, int mapWidth, int mapHeight)
        {
            switch (direction)
            {
                case Direction.up:
                    if (Position[0].y - 1 == 0)
                    {
                        return true;
                    }
                    return false;
                case Direction.right:
                    if (Position[0].x + 1 == mapWidth - 1)
                    {
                        return true;
                    }
                    return false;
                case Direction.down:
                    if (Position[0].y + 1 == mapHeight - 1)
                    {
                        return true;
                    }
                    return false;
                case Direction.left:
                    if (Position[0].x - 1 == 0)
                    {
                        return true;
                    }
                    return false;
                default:
                    return false;
            }
        }

        private void UpdateSnakePositionList(Direction direction, int mapWidth, int mapHeight, Point newHeadPointIfBorderHit, Point newHeadPointIfNormalMove)
        {
            if (CheckIfCollisionWithBoundary(direction, mapWidth, mapHeight) == true)
            {
                Position.Insert(0, newHeadPointIfBorderHit);
            }
            else
            {
                Position.Insert(0, newHeadPointIfNormalMove);
            }
            Position.RemoveAt(Position.Count - 1);
        }

        private bool CheckIfCollisionWithTail()
        {
            if (Position.Skip(1).Contains(Position[0]))
            {
                return true;
            }
            return false;
        }
        
        public void IncreaseLength()
        {
            Position.Add(Position[Position.Count - 1]);
        }
        public Point GetSnakeHead()
        {
            return Position[0];
        }
        public Point GetSnakeTail()
        {
            return Position[^1];
        }
    }
}
Board.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace SnakeGame
{

    public class Board
    {
        public readonly int Width;
        public readonly int Height;

        public Board(int width, int height)
        {
            Width = width;
            Height = height;
        }

        public void Draw(List<Point> snakePosition, Point previousSnakeTail, Point applePosition)
        {
            DrawBoard();

            DrawSnake(snakePosition, previousSnakeTail);

            DrawApple(applePosition);
        }

        private static void DrawApple(Point applePosition)
        {
            Console.SetCursorPosition(applePosition.x, applePosition.y);
            Console.BackgroundColor = ConsoleColor.Red;
            Console.Write("@");
            Console.BackgroundColor = ConsoleColor.Black;
        }

        private static void DrawSnake(List<Point> snakePosition, Point previousSnakeTail)
        {
            for (int i = 0; i < snakePosition.Count; i++)
            {
                Console.SetCursorPosition(snakePosition[i].x, snakePosition[i].y);
                if (i == 0)
                {
                    Console.ForegroundColor = ConsoleColor.Blue;
                    Console.Write("O");
                    Console.ForegroundColor = ConsoleColor.White;
                }
                else
                {
                    Console.Write("o");
                }
            }
            // Instead of redrawing whole Board just clear previous Snake tail
            Console.SetCursorPosition(previousSnakeTail.x, previousSnakeTail.y);
            Console.Write(" ");
        }

        private void DrawBoard()
        {
            Console.SetCursorPosition(0, 0);
            for (int i = 0; i < Width; i++)
            {
                Console.Write("#");
            }

            Console.WriteLine();
            for (int i = 0; i < Height - 2; i++)
            {
                Console.Write("#");
                Console.SetCursorPosition(Width - 1, i + 1);
                Console.WriteLine("#");
            }

            for (int i = 0; i < Width; i++)
            {
                Console.Write("#");
            }
        }
    }
}
Apple.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace SnakeGame
{
    public class Apple
    {
        public Point Position { get; private set; }
        public bool IsEaten { get; private set; }

        public Apple(int mapWidth, int mapHeight, List<Point> snakePosition)
        {
            IsEaten = false;

            SpawnApple(mapWidth, mapHeight, snakePosition);
        }

        private void SpawnApple(int mapWidth, int mapHeight, List<Point> snakePosition)
        {
            Random rand = new Random();
            Point applePosition;

            do
            {
                applePosition = new Point(rand.Next(1, mapWidth - 2), rand.Next(1, mapHeight - 2));
            } while (snakePosition.Contains(applePosition) != false);

            Position = applePosition;
        }

        public bool CheckIfEaten(Point SnakeHeadPosition)
        {
            if(SnakeHeadPosition == Position)
            {
                IsEaten = true;
                return true;
            }
            return false;
        }
    }
}
KeyboardManager.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace SnakeGame
{
    public static class KeyboardManager
    {
        public static Direction GetDirection()
        {
            if (Console.KeyAvailable)
            {
                ConsoleKeyInfo pressedKey = Console.ReadKey(true);

                ClearBuffer();

                switch (pressedKey.Key)
                {
                    case ConsoleKey.D:
                    case ConsoleKey.RightArrow:
                        return Direction.right;
                    case ConsoleKey.S:
                    case ConsoleKey.DownArrow:
                        return Direction.down;
                    case ConsoleKey.A:
                    case ConsoleKey.LeftArrow:
                        return Direction.left;
                    case ConsoleKey.W:
                    case ConsoleKey.UpArrow:
                        return Direction.up;
                    default:
                        return Direction.previousDirection;
                }
            }
            return Direction.previousDirection;
        }

        private static void ClearBuffer()
        {
            while (Console.KeyAvailable)
            {
                Console.ReadKey(true);
            }
        }
    }
}
Score.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace SnakeGame
{
    public class Score
    {
        private int points = 0;
        public void PrintScore(int mapHeight)
        {
            Console.SetCursorPosition(0, mapHeight);
            Console.Write($"Your score: {points}");
        }

        public void IncreasePoints()
        {
            points++;
        }

        public int GetPoints()
        {
            return points;
        }
    }
}
Messages.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace SnakeGame
{
    public static class Messages
    {
        public static void GameOverMessage(int mapWidth, int mapHeight, int score)
        {
            Console.SetCursorPosition(mapWidth / 4, mapHeight / 2);
            Console.ForegroundColor = ConsoleColor.Green;
            Console.WriteLine("GAME OVER");
            Console.SetCursorPosition(mapWidth / 4, mapHeight / 2 + 1);
            Console.WriteLine($"Your score was: {score}");
            Console.ForegroundColor = ConsoleColor.White;
        }
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Program.cs. Well done! Rare in the extreme to see good OO with appropriate abstraction level, and decoupling the application from the environment entry point - Program.cs. \$\endgroup\$ Commented May 13, 2023 at 21:14

1 Answer 1

6
\$\begingroup\$

Program.cs

Well done! Rare in the extreme to see good OO with appropriate abstraction level, and decoupling the application from the environment entry point.


No public setters

public class Game
{
    public Board Board { get; protected set; }
    public Snake Snake { get; protected set; }
    public Apple Apple { get;  protected set; }
    public Score Score { get;  protected set; }
    

A good class exposes functionality and hides state. The way the code is already written you doesn't need them. NEVER publicly expose internal state.


Not this:

       if (Position[0].y - 1 == 0)
                {
                    return true;
                }
                return false;
                

This:

    return (Position[0].y - 1 == 0);
    

also the operator overload methods:

       return (p1.x == p2.x && p1.y == p2.y)
       

Easy Complexity Reduction Technique

Return immediately on terminating conditions. Use the ternary operator. This reduces technical complexity scores because those are mainly about the total possible branches.

private Direction UpdateCurrentDirection(Direction newDirection)
{
    if (newDirection == Direction.previousDirection) return currentDirection;
        
    if (Position.Count == 1)  return newDirection;

    return CheckIfOppositeDirection(newDirection)) ? 
              currentDirection : newDirection;
}

One line if w/out brackets super-sizes comprehension speed. The secret to getting away with that is to leave white space above & below.

BEFORE:

private Direction UpdateCurrentDirection(Direction newDirection)
{
    if (newDirection != Direction.previousDirection)
    {
        if (Position.Count == 1)
        { 
            return newDirection;
        }
        if (CheckIfOppositeDirection(newDirection) == true)
        {
            return currentDirection;
        }
        else
        {
            return newDirection;
        }
    }
    else
    {
        return currentDirection;
    }
}

as long as true is true this truthy true is true

 public void Start()
    {
        // ...
         while (true) {
             // ...
             bool isBodyCollision = Snake.Move(direction, Board.Width, Board.Height);
            if (isBodyCollision) break;
           // ...
        }

Name the rules controlling game play. Move isBodyCollision declaration out of the loop.

      bool isBodyCollision = false
       while (! isBodyCollision) {
             // ...
            isBodyCollision = Snake.Move(direction, Board.Width, Board.Height);
            if (isBodyCollision) continue;
           // ...
        }
 

Let the loop conditional control execution. It tends to help prevent temperal coupling as code changes and complexity increases.


Leaky Abstraction

I like the variable name "Position"

public class Snake {  ...  Position = new List<Point>();  ... }

Playing the game we talk about the Snake's position, not a collection of positions. But then the abstraction leaks its implementation somewhat with a method name:

UpdateSnakePositionList

Change that to

UpdateSnakePosition
Abstracting - It's Turtles All the Way Down

Each layer of code structure, method, and class is an opportunity for abstraction into actual Snake Game terminology, concepts, and game play. As a general rule use names and verbs abstracting away, not hanging onto, actual code implementaiton.

Below requires reading the loop code and often reading deeper to make sure I understand what is "true"ly going on:

while (true) {
   // ....
}

All that effort goes away when:

while (! isBodyCollision) {
   // ....
}

Class name gives context to methods

This: Draw Not: DrawBoard

Concise reading enjoyment:

    Board snakeMaze = new Board(...);
    snakeMaze.Draw();
    

switch case & fall-through formatting

Always put space between a case break/return and the next case. Fall-through cases in particular stand out much better.

       case ConsoleKey.D:
       case ConsoleKey.RightArrow:
             return Direction.right;
        
        case ConsoleKey.S:
        case ConsoleKey.DownArrow:
              return Direction.down;
                
        case ConsoleKey.A:
        

Check 'check' in method names

"Check" is such a generic term that it means nothing. But I'll admit I can't help myself sometimes. Try very hard to replace "check" with something else. It is easier to fix when returning boolean, prefix with "is"

  • CheckIfEaten - IsEaten
  • CheckIfOppositeDirection - ChangedDirection
  • CheckIfCollisionWithBoundary - IsBoundryCollision

Concise reading enjoyment:

if ( Apple.IsEaten())
if (ChangedDirection())

Get is a given

Methods like this: GetSnakeHead(). After about 7,602 GET-prefixed methods, you will realize it is superfluous. SnakeHead() is better. Make it a property - Snake.Head is better still.

Concise reading enjoyment:

  if (Apple.IsEaten(Snake.Head))
  

Uh, oh. I now see a problem. Not kidding - I thought of it only after I wrote that. The code feels "backwards" and I think "inversion of control"


Inversion of Control (IOC)

 public class Start {
     .....
       if (Apple.CheckIfEaten(Snake.GetSnakeHead())) 
     ....
} // Start

The Apple should not be responsible for determining if the snake ate it. That's for the snake to do. Due to the "inverted" (backward, inside-out) referencing (dependencies) the Start class must know Snake internal details, head vs tail vs otherParts. Apple also, for it's part.

In other words Start must know how the snake eats because Snake does not know how to eat - it has no eating behavior (method). If you have ever experienced a waiter suddenly shoving any part of your body into a plate of food, then you understand the issue here.

Concise reading enjoyment:

    if (Snake.Eats(Apple)) 

Decouple the game from the UI

There should be a new class that makes console calls for UI interaction. There will be no console code anywhere in the Snake Game classes. Then make Game class methods/properties for score, board, etc. The UI class talks to Game object and no others. This is the principle of least knowledge.

Suggestion: Board.ToString() returns the board/grid as a string with/n separating rows. Same thing for Score.ToString() and any other UI bound output.

Then Game has the interface used by UI objects.

public class Game {
    public string Score() { return Score.ToString(); }
    public string Board() { return Board.ToString(); } 
    public override string  ToString() {  return Score() + `/n/n` +  Board();   }  
}

Concise reading enjoyment:

 console.WriteLine(mySnakeGame);     // implicit ToString call
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thank you sir! Your in-depth code analysis is truly amazing. Thanks for every single piece of advice you've just mentioned ;-) \$\endgroup\$ Commented May 14, 2023 at 15:36
  • 2
    \$\begingroup\$ "Decouple the game from the UI" – One of the most important advantages of doing this is that it allows you to perform automatic testing of your game without having to play it and without having to scrape the GUI to check the result. \$\endgroup\$ Commented May 16, 2023 at 22:25

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.