Object-oriented Snake game

Question

Right now I have a snake game that is working but I need a way to make my code look better and work better.

A method I named GameSense is very long and I need a way to make it more object-oriented. I want to use enums.

This is what my teacher sent back to me:

The game is very nice and nicely working tail! The test you have also follows the right idea but uses an UpdatePosition method that you have not implemented. I'm also thinking about your class pixel - you do not use it for anything in the rest of the game. You should use it instead of e.g. berryx, berryy. What I am trying to access is more use of object orientation and division into methods, and not a single long method like GameSense right now. Also want to see you use enum / bool instead of strings for e.g. the variables movement and buttonpressed. However, I'm glad you got an interface on a corner

As you can see… I feel pretty suck right now.
What do I need to do here in order to make the program be more object-oriented?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Threading;

namespace Snake
{
    /// <summary>
    /// Start class, everything goes from here.
    /// Made by Karwan!
    /// </summary>
   public class Starter : InterF
    { 
        public static object Key { get; private set; }

        /// <summary>
        /// Main method.
        /// </summary>
        /// <param name="args"></param>
        static void Main(string[] args)
        {
            //Making the text green and also given a player 
            //options to chose from. 
            Console.ForegroundColor = ConsoleColor.Green;
            bool loopC = true;
            while (loopC)
            {
                string mystring = null; Console.WriteLine("Welcome to Snake.\nPress S+Enter to start or press Q+Enter to exit.");
                mystring = Console.ReadLine();
                switch (mystring)
                {
                    case "Q":
                        Environment.Exit(0);
                        break;
                    case "S":
                        Console.WriteLine("Starting game!");
                        System.Threading.Thread.Sleep(4000);
                        loopC = false;
                        break;
                    default:
                        Console.WriteLine("Invalid Entry.. Try again.");
                        break;
                }

            }

            //Call for GameSense if
            //they want to play.
            GameSense(); 

        }


      
        /// <summary>
        /// Game object!
        /// </summary>
        public static void GameSense()

        {
            //Game console height/width.
            Console.WindowHeight = 30;
            Console.WindowWidth = 70;
            int screenwidth = Console.WindowWidth;
            int screenheight = Console.WindowHeight;
            Random randomnummer = new Random();
            //Lenght of tail == current score.
            int score = 2;
            int gameover = 0;
            //Gather positions from pixel class.
            pixel positions = new pixel();
            positions.xpos = screenwidth / 2;
            positions.ypos = screenheight / 2;
            positions.Black = ConsoleColor.Red;
            string movement = "RIGHT";
            //Position manegment.
            List<int> xpos = new List<int>();
            List<int> ypos = new List<int>();
            int berryx = randomnummer.Next(0, screenwidth);
            int berryy = randomnummer.Next(0, screenheight);
            //Time manegment.
            DateTime time1 = DateTime.Now;
            DateTime time2 = DateTime.Now;
            string buttonpressed = "no";

            
            //Draw world from GameWorld.cs.
            GameWorld.DrawBorder(screenwidth, screenheight);

            while (true)
            {
                GameWorld.ClearConsole(screenwidth, screenheight);
                if (positions.xpos == screenwidth - 1 || positions.xpos == 0 || positions.ypos == screenheight - 1 || positions.ypos == 0)
                {
                    gameover = 1;
                }

                Console.ForegroundColor = ConsoleColor.Green;
                if (berryx == positions.xpos && berryy == positions.ypos)
                {
                    score++;
                    berryx = randomnummer.Next(1, screenwidth - 2);
                    berryy = randomnummer.Next(1, screenheight - 2);
                }
                for (int i = 0; i < xpos.Count(); i++)
                {
                    Console.SetCursorPosition(xpos[i], ypos[i]);
                    Console.Write("*");
                    if (xpos[i] == positions.xpos && ypos[i] == positions.ypos)
                    {
                        gameover = 1;
                    }
                }
                if (gameover == 1)
                {
                    break;
                }
                Console.SetCursorPosition(positions.xpos, positions.ypos);
                Console.ForegroundColor = positions.Black;
                Console.Write("*");
                //Food color & position.
                Console.SetCursorPosition(berryx, berryy);
                Console.ForegroundColor = ConsoleColor.Cyan;
                Console.Write("*");
                Console.CursorVisible = false;
                time1 = DateTime.Now;
                buttonpressed = "no";
                while (true)
                {
                    time2 = DateTime.Now;
                    if (time2.Subtract(time1).TotalMilliseconds > 500) { break; }
                    if (Console.KeyAvailable)
                    {
                        ConsoleKeyInfo info = Console.ReadKey(true);
                        //Connecting the buttons to the x/y movments. 
                        if (info.Key.Equals(ConsoleKey.UpArrow) && movement != "DOWN" && buttonpressed == "no")
                        {
                            movement = "UP";
                            buttonpressed = "yes";
                        }
                        if (info.Key.Equals(ConsoleKey.DownArrow) && movement != "UP" && buttonpressed == "no")
                        {
                            movement = "DOWN";
                            buttonpressed = "yes";
                        }
                        if (info.Key.Equals(ConsoleKey.LeftArrow) && movement != "RIGHT" && buttonpressed == "no")
                        {
                            movement = "LEFT";
                            buttonpressed = "yes";
                        }
                        if (info.Key.Equals(ConsoleKey.RightArrow) && movement != "LEFT" && buttonpressed == "no")
                        {
                            movement = "RIGHT";
                            buttonpressed = "yes";
                        }
                    }
                }
                //Giving the connections value
                //to change x/y to make the movment happen.
                xpos.Add(positions.xpos);
                ypos.Add(positions.ypos);
                switch (movement)
                {
                    case "UP":
                        positions.ypos--;
                        break;
                    case "DOWN":
                        positions.ypos++;
                        break;
                    case "LEFT":
                        positions.xpos--;
                        break;
                    case "RIGHT":
                        positions.xpos++;
                        break;
                }
                if (xpos.Count() > score)
                {
                    xpos.RemoveAt(0);
                    ypos.RemoveAt(0);
                }
            }
            Console.SetCursorPosition(screenwidth / 5, screenheight / 2);
            Console.WriteLine("Game over, Score: " + score);
            Console.SetCursorPosition(screenwidth / 5, screenheight / 2 + 1);
            System.Threading.Thread.Sleep(1000);
            restart();
            
        }

        /// <summary>
        /// Restarter.
        /// </summary>
        public static void restart()
        {
        
            string Over = null; Console.WriteLine("\nWould you like to start over? Y/N");
            bool O = true;

            while (O)
            {
                Over = Console.ReadLine();
                switch (Over)
                {
                    case "Y":
                        Console.WriteLine("\nRestarting!");
                        System.Threading.Thread.Sleep(2000);
                        break;
                    case "N":
                        Console.WriteLine("\nThank you for playing!");
                        Environment.Exit(0);
                        break;
                    default:
                        Console.WriteLine("Invalid Entry.. Try again.");
                        break;
                }
            }

        }
        /// <summary>
        /// Set/get pixel position.
        /// </summary>
       public class pixel
        {
            public int xpos { get; set; }
            public int ypos { get; set; }
            public ConsoleColor Black { get; set; }
        }
    }
}

My test class

using Microsoft.VisualStudio.TestTools.UnitTesting;
using NUnit.Framework;
using Snake;
using System;
using System.Collections.Generic;
using System.Text;
using static Snake.Starter;

namespace Snake.Tests
{
    [TestClass()]
    public class StarterTests
    {
        /// <summary>
        /// Testing the movment.
        /// </summary>
        [TestMethod()]
        public void test()
        {
           
           var pos = new pixel();
            pos.xpos = 10;
            pos.ypos = 10;

            // this is the operation:
            UpdatePosition(pos, "UP");

            // check the results
            NUnit.Framework.Assert.That(pos.xpos, Is.EqualTo(10));
            NUnit.Framework.Assert.That(pos.ypos, Is.EqualTo(9));
        }

        /// <summary>
        /// Updating pixel class.
        /// </summary>
        /// <param name="pos"></param>
        /// <param name="v"></param>
        private void UpdatePosition(pixel pos, string v)
        {
            throw new NotImplementedException();
        }
    }
}

This is just the gameworld:

using System;
using System.Linq;

public class GameWorld 
{


    /// <summary>
    /// Clear console for snake.
    /// </summary>
    /// <param name="screenwidth"></param>
    /// <param name="screenheight"></param>
    public static void ClearConsole(int screenwidth, int screenheight)
    {
        var blackLine = string.Join("", new byte[screenwidth - 2].Select(b => " ").ToArray());
        Console.ForegroundColor = ConsoleColor.Black;
        for (int i = 1; i < screenheight - 1; i++)
        {
            Console.SetCursorPosition(1, i);
            Console.Write(blackLine);
        }
    }

    /// <summary>
    /// Draw boared.
    /// </summary>
    /// <param name="screenwidth"></param>
    /// <param name="screenheight"></param>
    public static void DrawBorder(int screenwidth, int screenheight)
    {
        var horizontalBar = string.Join("", new byte[screenwidth].Select(b => "■").ToArray());

        Console.SetCursorPosition(0, 0);
        Console.Write(horizontalBar);
        Console.SetCursorPosition(0, screenheight - 1);
        Console.Write(horizontalBar);

        for (int i = 0; i < screenheight; i++)
        {
            Console.SetCursorPosition(0, i);
            Console.Write("■");
            Console.SetCursorPosition(screenwidth - 1, i);
            Console.Write("■");
        }
    }

}

Answer 1

OOP - Grouping data that belongs together

I'm also thinking about your class pixel - you do not use it for anything in the rest of the game. You should use it instead of e.g. berryx, berryy.

You are storing X and Y positions in unrelated lists:

List<int> xpos = new List<int>();
List<int> ypos = new List<int>();
int berryx = randomnummer.Next(0, screenwidth);
int berryy = randomnummer.Next(0, screenheight);

But earlier, you were grouping a position (i.e. an XY coordinate) in a single object:

pixel positions = new pixel();
positions.xpos = screenwidth / 2;
positions.ypos = screenheight / 2;

Your teacher is essentially telling you to do the same thing for all X and Y coordinates.

I'm going to introduce a new class here called Position. It's like your Pixel class, but without specifying a color. The goal of the Position class is to represent a single (X,Y) coordinate:

public class Position
{
    public int X { get; set; }
    public int Y { get; set; }
}

Now that you have this class, you can start using it in all places where you are currently using separate X and Y values:

// Instead of:

List<int> xpos = new List<int>();
List<int> ypos = new List<int>();  

// Use:

List<Position> positions = new List<Position>();  

// Instead of:

int berryx = randomnummer.Next(0, screenwidth);
int berryy = randomnummer.Next(0, screenheight);

// Use:

Position berryPosition = new Position()
{
    X = randomnummer.Next(0, screenwidth),
    Y = randomnummer.Next(0, screenheight)
}; 

Note also that the Pixel class can be adjusted to reuse this Position:

public class Pixel
{
    public Position Position { get; set; }
    public ConsoleColor Color { get; set; }
}

I'm going to leave the further adjustments (i.e. how you access this data later on) as an exercise for you. It's no different from how you were accessing your Pixel data already.

---

OOP - Class methods

Other than storing data values, classes can have methods. Class methods are great for housing any logic that belongs to the object.

For example, berries should be able to decide how they generate their position. Right now, you're just using a total random, but that could change over time. For example, berries might not want to spawn on borders, or next to their previous position.
Whatever the reason, the main takeaway here is that it'd be practical to have the berry contain the "berry position generating logic".

To keep things simple, we're going to assume that the same berry doesn't change position when it is eaten, but rather that you always make a new Berry (hint!) whenever the previous one gets eaten. Therefore, the position of the new berry can be decided in the class constructor.

I'm going to not do something we could do. Try and spot it. I'll correct it later.

public class Berry
{
    public int X { get; set; }
    public int Y { get; set; }

    public Berry(int screenWidth, int screenHeight)
    {
        var rand = new Random();
        this.X = GenerateX(rand, screenwidth);
        this.Y = GenerateY(rand, screenHeight);
    }

    private int GenerateX(Random rand, int screenWidth)
    {
         return rand.Next(0, screenWidth); 
    }

    private int GenerateY(Random rand, int screenHeight)
    {
         return rand.Next(0, screenHeight); 
    }
}

Did you spot the thing we could improve? Think back to the previous section.

We created a Position class specifically to represent XY coordinates. We should be reusing that logic here!

public class Berry
{
    public Position Position { get; private set; }

    public Berry(int screenWidth, int screenHeight)
    {
        var rand = new Random();
        this.Position = GeneratePosition(rand, screenWidth, screenHeight);
    }

    private int GeneratePosition(Random rand, int screenWidth, int screenHeight)
    {
         return new Position()
         {
             X = rand.Next(0, screenWidth),
             Y = rand.Next(0, screenHeight)
         }; 
    }
}

There's another berry-specific piece of logic in your code: drawing it on the screen. Berries have their own color. This belongs to the Berry class.

Note that I'm omitting the Berry code from the previous snippet to keep it focused, but these should of course be merged together in the real code.

public class Berry
{
    // ...

    public ConsoleColor Color = ConsoleColor.Cyan;

    public void Draw()
    {
        Console.SetCursorPosition(this.Position.X, this.Position.Y);
        Console.ForegroundColor = this.Color;
        Console.Write("*");
        Console.CursorVisible = false;
    }
}

Notice how we've relied on the fact that the Berry object already knows its position and its color, so we were able to write our code to just fetch that information.

This opens the door to a neat little improvement. Just like we randomize the berry's position, why not randomize its color? If you look back at the Draw() logic, you'll see that it just uses the color in this.Color. So if we change the value of this.Color, the printed character will change color (without us needing the change the Draw() method itself).

I'm going to limit the possible range of colors to a list that we pre-select. That's better than a wild random. Also, take note of this StackOverflow answer with many solutions on how to randomly select a color.

This is the combined logic of everything we discussed:

public class Berry
{
    public Position Position { get; private set; }
    public ConsoleColor Color { get; private set; }

    private readonly ConsoleColor[] _allowedColors = new ConsoleColor[]
    {
        ConsoleColor.Cyan, ConsoleColor.Yellow, ConsoleColor.Red, ConsoleColor.Green
    };

    public Berry(int screenWidth, int screenHeight)
    {
        var rand = new Random();
        this.Position = GeneratePosition(rand, screenWidth, screenHeight);
        this.Color = GenerateColor(rand);
    }

    private int GeneratePosition(Random rand, int screenWidth, int screenHeight)
    {
         return new Position()
         {
             X = rand.Next(0, screenWidth),
             Y = rand.Next(0, screenHeight)
         }; 
    }

    private ConsoleColor GenerateColor(Random rand)
    {
        return _allowedColors.ElementAt(random.Next(_allowedColors .Length));
    }

    public void Draw()
    {
        Console.SetCursorPosition(this.Position.X, this.Position.Y);
        Console.ForegroundColor = this.Color;
        Console.Write("*");
        Console.CursorVisible = false;
    }
}

This is an object-oriented berry. Now, your game logic has to minimally interact with the berry:

var berry = new Berry(Console.WindowWidth, Console.WindowHeight);

berry.Draw();

Everything else is contained in the Berry class. Hint: that's why they call this kind of coding style "encapsulation". The berry logic is contained in the Berry class and hidden from sight so the rest of the codebase doesn't get distracted by it.

---

Magic strings

Also want to see you use enum / bool instead of strings for e.g. the variables movement and buttonpressed.

Strings are nice and readable, but they are also not very memory efficient (compared to other data types), and they allow for a lot of ambiguity, e.g. how "right" and "Right" are two different values.

You should use data types that match what you're trying to do.

buttonpressed is clearly a boolean value. The easiest way to think of a boolean is to think of a light switch. It only has two possible positions. Whether that's "yes/no", "on/off", "true/false", "alive/dead", ... is semantical and not relevant. In C#, you just use true and false

bool buttonPressed = false;

if(buttonPressed)
    Console.WriteLine("This WON'T be printed because it's false");

buttonPressed = true;

if(buttonPressed)
    Console.WriteLine("This WILL be printed because it's true");

I suspect your teacher will have given you study material about data types such as booleans, so I suggest you revise that material.

movement is different. Here, you're not dealing with something that is easily expressed using a known data type. You're dealing with a closed list of a few options, so an enum is appropriate here.

public enum Direction { Up, Down, Left, Right }

The intention of your code will remain the same, but you'll be using better data types that better suit your needs:

//Connecting the buttons to the x/y movments. 
if (info.Key.Equals(ConsoleKey.UpArrow) && movement != Direction.Down && !buttonpressed)
{
    movement = Direction.Up;
    buttonpressed = true;
}
if (info.Key.Equals(ConsoleKey.DownArrow) && movement != Direction.Up && !buttonpressed)
{
    movement = Direction.Down;
    buttonpressed = true;
}
if (info.Key.Equals(ConsoleKey.LeftArrow) && movement != Direction.Right && !buttonpressed)
{
    movement = Direction.Left;
    buttonpressed = true;
}
if (info.Key.Equals(ConsoleKey.RightArrow) && movement != Direction.Left && !buttonpressed)
{
    movement = Direction.Right;
    buttonpressed = true;
}

I left buttonpressed in for now, to show you the syntax. However, you don't actually need it here. If you had used if else instead of if, you would've automatically been sure that only one possible outcome would be reached, and not multiple.

if (info.Key.Equals(ConsoleKey.UpArrow) && movement != Direction.Down)
{
    movement = Direction.Up;
}
else if (info.Key.Equals(ConsoleKey.DownArrow) && movement != Direction.Up)
{
    movement = Direction.Down;
}
else if (info.Key.Equals(ConsoleKey.LeftArrow) && movement != Direction.Right)
{
    movement = Direction.Left;
}
else if (info.Key.Equals(ConsoleKey.RightArrow) && movement != Direction.Left)
{
    movement = Direction.Right;
}