HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Simple console Snake game following GRASP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplegraspsnakefollowinggameconsole

Problem

I would like to know if there is anything that I missed following the GRASP patterns.

The Main Controller class:

```
using System;
using System.Collections.Generic;
using System.Diagnostics;

namespace ConsoleApplication1
{
public class SnakeGame
{
private bool GameIsPlaying { get; set; }
private bool AppleEaten { get; set; }
private bool Paused { get; set; }

public List TailCoordsList;
private PlayField PlayField { get; set; }
private Snake Player { get; set; }
private Stopwatch Tick { get; set; }
private GameRenderer Renderer { get; set; }
private Direction Direction { get; set; }
private Apple Apple { get; set; }
private Input Input { get; set; }

private void Initiliaze()
{
GameIsPlaying = true;
Paused = false;
AppleEaten = false;

TailCoordsList = new List();

Apple = new Apple();
PlayField = new PlayField(TailCoordsList);
Apple.SetAppleOnField(TailCoordsList, PlayField.Width, PlayField.Height);

Tick = new Stopwatch();
Renderer = new GameRenderer();

Player = new Snake(TailCoordsList);
Direction = new Direction();

Tick.Start();
UpdatePlayField();
}

private void UpdatePlayField()
{
while (GameIsPlaying)
{
OnKeyDown(); // Input logic

if (!Paused) // Game logic
{
if (Tick.ElapsedMilliseconds < 100) continue; // Tick ensures that each update will happen in constant time.
Tick.Restart();

Player.GetNextPosition(TailCoordsList);
Player.SwitchDirection

Solution

Sort/remove usings

it's not a huge deal, just adds to noise to the source, but make it a habbit to remove unused usings in your source code. I prefer to use Producitiviy tools (a free extension for VS) you can right click on the solution and remove and sort all unused usings.

Apple.cs

I feel like your code is doing some strange things. Apple extends PlayField which extends SnakeGame. Why? I commented out the extends PlayField part and the game worked just fine.
You create a new instance of Random and FieldRenderer in the file, but initialize AppleCoord in the constructor? It would be better to stay consistant and initialize all 3 in the constructor, however Random is typically best if you make it a static field. FieldRenderer is a concrete implementation of Renderer, but you only use the abstract method, instead pass in your Renderer in the constructor instead of initializing it. AppleCoord is a bad name, and should be spelled out. Also you allow it to be set! It would be better to make it readonly and make a public getter for collision detection later. Lastly you pass in a List it would be better if you just passed in IEnumerable. Remember to favor depending on the abstract (or even bettter the interface). Why does it return bool? You don't use the value. have it return void instead.

public class Apple
{
    private static readonly Random _random = new Random();
    public Coord AppleCoordinate { get { return _appleCoordinate; } }

    private readonly Renderer _renderer;
    private readonly Coord _appleCoordinate;
    private readonly int _playFieldWidth;
    private readonly int _playFieldHeight;

    public Apple(Renderer renderer, PlayField playField)
    {
        _renderer = renderer;
        _appleCoordinate = new Coord();
        _playFieldWidth = playField.Width;
        _playFieldHeight = playField.Height;
    }

    public void SetAppleOnField(IEnumerable tailCoords)
    {
        var isValidAppleCoordinate = false;
        while (!isValidAppleCoordinate)
        {
            _appleCoordinate.X = _random.Next(0, _playFieldWidth);
            _appleCoordinate.Y = _random.Next(0, _playFieldHeight);

            isValidAppleCoordinate = tailCoords.All(i => i.X != _appleCoordinate.X || i.Y != _appleCoordinate.Y);
        }
        _renderer.DrawCharAtLoc(_appleCoordinate, '

Renderers

Renderer does not need to extend SnakeGame (same reason as Apple). There is some duplicate code between you different renderers so make a protected method that you renderes call. One thing that baffles me is the FieldRenderer::InitConsole. Why is it in FieldRenderer? That violates SRP. Move it somewhere else.. like SnakeGame. That would make more sense. DrawCharAtLoc is a bad name for a method. Since your Renderer "Renders" maybe it would be better called Render.

public abstract class Renderer
{
    public abstract void Render(Coord coord, char c);

    protected void Draw(Coord coord, char c, ConsoleColor color)
    {
        Console.ForegroundColor = color;
        Console.SetCursorPosition(coord.X, coord.Y);
        Console.Write(c);
    }
}

public class GameRenderer : Renderer
{
    public override void Render(Coord coord, char c)
    {
        Draw(coord, c, ConsoleColor.Yellow);
    }
}

public class FieldRenderer : Renderer
{
    public override void Render(Coord coord, char c)
    {
        Draw(coord, c, ConsoleColor.Green);
    }
}


Direction.cs & Input.cs

Why does Direction extend Snake? Again I commented it out and the game works just fine.
These two classes are tightly coupled. That might not be a bad thing, but I would rethink this. Take a step back and think: "What goes in a direction? A snake. Who should tell the snake what direction to go? The input. What if the snake can't do what was input? I would say Ignore the input" with that in mind. Lets just check if there is input, and if there is lets suggest to the snake what direction to go. Then we can remove the dependency on Direction in the Input class and it can stand on its own.

```
internal class Input
{
private readonly ConsoleKeyInfo _keyInfo;
public bool hasInput { get; set; }

public Input()
{
_keyInfo = Console.ReadKey(true);
}

public bool KeyAvalible { get { return !Console.KeyAvailable; } }

public bool IsPause { get { return _keyInfo.Key == ConsoleKey.Spacebar; } }

public bool IsQuit { get { return _keyInfo.Key == ConsoleKey.Escape; } }

public bool IsUp { get { return _keyInfo.Key == ConsoleKey.UpArrow; } }

public bool IsDown { get { return _keyInfo.Key == ConsoleKey.DownArrow; } }

public bool IsRight { get { return _keyInfo.Key == ConsoleKey.RightArrow; } }

public bool IsLeft { get { return _keyInfo.Key == ConsoleKey.LeftArrow; } }
}

//SnakeGame.cs
private void OnKeyDown()
{
input = new Input();
if (!input.HasInput) return;
if (input.IsQuit)
GameIsPlaying = false;
else); } }


Renderers

Renderer does not need to extend SnakeGame (same reason as Apple). There is some duplicate code between you different renderers so make a protected method that you renderes call. One thing that baffles me is the FieldRenderer::InitConsole. Why is it in FieldRenderer? That violates SRP. Move it somewhere else.. like SnakeGame. That would make more sense. DrawCharAtLoc is a bad name for a method. Since your Renderer "Renders" maybe it would be better called Render.

%%CODEBLOCK_1%%

Direction.cs & Input.cs

Why does Direction extend Snake? Again I commented it out and the game works just fine.
These two classes are tightly coupled. That might not be a bad thing, but I would rethink this. Take a step back and think: "What goes in a direction? A snake. Who should tell the snake what direction to go? The input. What if the snake can't do what was input? I would say Ignore the input" with that in mind. Lets just check if there is input, and if there is lets suggest to the snake what direction to go. Then we can remove the dependency on Direction in the Input class and it can stand on its own.

```
internal class Input
{
private readonly ConsoleKeyInfo _keyInfo;
public bool hasInput { get; set; }

public Input()
{
_keyInfo = Console.ReadKey(true);
}

public bool KeyAvalible { get { return !Console.KeyAvailable; } }

public bool IsPause { get { return _keyInfo.Key == ConsoleKey.Spacebar; } }

public bool IsQuit { get { return _keyInfo.Key == ConsoleKey.Escape; } }

public bool IsUp { get { return _keyInfo.Key == ConsoleKey.UpArrow; } }

public bool IsDown { get { return _keyInfo.Key == ConsoleKey.DownArrow; } }

public bool IsRight { get { return _keyInfo.Key == ConsoleKey.RightArrow; } }

public bool IsLeft { get { return _keyInfo.Key == ConsoleKey.LeftArrow; } }
}

//SnakeGame.cs
private void OnKeyDown()
{
input = new Input();
if (!input.HasInput) return;
if (input.IsQuit)
GameIsPlaying = false;
else

Code Snippets

public class Apple
{
    private static readonly Random _random = new Random();
    public Coord AppleCoordinate { get { return _appleCoordinate; } }

    private readonly Renderer _renderer;
    private readonly Coord _appleCoordinate;
    private readonly int _playFieldWidth;
    private readonly int _playFieldHeight;

    public Apple(Renderer renderer, PlayField playField)
    {
        _renderer = renderer;
        _appleCoordinate = new Coord();
        _playFieldWidth = playField.Width;
        _playFieldHeight = playField.Height;
    }

    public void SetAppleOnField(IEnumerable<Coord> tailCoords)
    {
        var isValidAppleCoordinate = false;
        while (!isValidAppleCoordinate)
        {
            _appleCoordinate.X = _random.Next(0, _playFieldWidth);
            _appleCoordinate.Y = _random.Next(0, _playFieldHeight);

            isValidAppleCoordinate = tailCoords.All(i => i.X != _appleCoordinate.X || i.Y != _appleCoordinate.Y);
        }
        _renderer.DrawCharAtLoc(_appleCoordinate, '$');
    }
}
public abstract class Renderer
{
    public abstract void Render(Coord coord, char c);

    protected void Draw(Coord coord, char c, ConsoleColor color)
    {
        Console.ForegroundColor = color;
        Console.SetCursorPosition(coord.X, coord.Y);
        Console.Write(c);
    }
}

public class GameRenderer : Renderer
{
    public override void Render(Coord coord, char c)
    {
        Draw(coord, c, ConsoleColor.Yellow);
    }
}

public class FieldRenderer : Renderer
{
    public override void Render(Coord coord, char c)
    {
        Draw(coord, c, ConsoleColor.Green);
    }
}
internal class Input
{
    private readonly ConsoleKeyInfo _keyInfo;
    public bool hasInput { get; set; }

    public Input()
    {
        _keyInfo = Console.ReadKey(true);
    }

    public bool KeyAvalible { get { return !Console.KeyAvailable; } }

    public bool IsPause { get { return _keyInfo.Key == ConsoleKey.Spacebar; } }

    public bool IsQuit { get { return _keyInfo.Key == ConsoleKey.Escape; } }

    public bool IsUp { get { return _keyInfo.Key == ConsoleKey.UpArrow; } }

    public bool IsDown { get { return _keyInfo.Key == ConsoleKey.DownArrow; } }

    public bool IsRight { get { return _keyInfo.Key == ConsoleKey.RightArrow; } }

    public bool IsLeft { get { return _keyInfo.Key == ConsoleKey.LeftArrow; } }
}

//SnakeGame.cs
private void OnKeyDown()
{
    input = new Input();
    if (!input.HasInput) return;
    if (input.IsQuit)
        GameIsPlaying = false;
    else if (input.IsPause)
        Paused = !Paused;
    else if (input.IsUp)
        _snake.SuggestGoingUp();
    else if (input.IsDown)
        _snake.SuggestGoingDown();
    else if (input.IsLeft)
        _snake.SuggestGoingLeft();
    else if (input.IsRight)
        _snake.SuggestGoingRight();
}

Context

StackExchange Code Review Q#108677, answer score: 4

Revisions (0)

No revisions yet.