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

Text-based rover game

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

Problem

I've made a small "rover game" similar to this one, except made in C#, using OOP, and prettier menus. Here's how it works:

There is one Rover class which is instantiated into an array containing all rovers. This class contains certain actions that a rover might need like, MoveRover, or RoverDead. There is also a RoverManager class where actions such as updating rovers are executed by looping through the array where the rover data is stored. This is also where rover addition and deletion is managed as well.

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

namespace Rovers
{
/ A class for creating a rover /
class Rover
{
public long x;
public long y;
public string name;
public int lifeTime;

public Rover(long x, long y, string name, int lifeTime)
{
this.x = x;
this.y = y;
this.name = name;
this.lifeTime = lifeTime;
}

/ Update the rover lifetime /
public void UpdateRoverLifeTime(int xChange, int yChange)
{
this.lifeTime -= Math.Abs(xChange - yChange);
}

/ Move the rover a desired amount /
public void MoveRover(int xChange, int yChange)
{
if (xChange = -5 && yChange >= -5)
{
this.x += xChange;
this.y += yChange;
UpdateRoverLifeTime(xChange, yChange);
}
else
{
Console.WriteLine("\nPOSITION CHANGE MUST BE = -5.");
}
}

/ Get input for position changes of the rover /
public void GetRoverPositionChange()
{
Console.WriteLine("\n-- Rover Position Changer Menu. --");

Console.Write("xChange: ");
int xChange = Int32.Parse(Console.ReadLine());

Console.Write("yChange: ");
int yChange = Int32.Parse(Console.ReadLine());

MoveRover(xChange, y

Solution

public List roverList = new List();


This field can (and should) be made private. We don't want strangers messing around with our rovers!

While we're at it, let's change the name from roverList to rovers -- we don't need the name to reflect the fact that it's a list of rovers.

Finally, we can make the field readonly.

public void DestroyRovers()
{
    for (int n = roverList.Count-1; n >= 0; n--)
    {
        Rover rover = roverList[n];

        if (rover.RoverDead())
        {
            roverList.Remove(rover);
        }
    }
}


Here we're removing all the dead rovers from the list. There's a method RemoveAll that can do this for us.

public void DestroyRovers()
{
    this.rovers.RemoveAll(rover => rover.RoverDead());
}


rover.RoverDead() looks strange to me; I think IsDead would be a better name. IsDead is a good candidate for a property since we're just returning the result of a quick comparison, so let's change this

public bool RoverDead()
{
    return this.lifeTime > 0;
}


to this

public bool IsDead
{
    get { return this.lifeTime > 0; }
}


Now our method looks better:

public void DestroyRovers()
{
    this.rovers.RemoveAll(rover => rover.IsDead);
}


public void UpdateRovers()
{
    for (int n = roverList.Count-1; n >= 0; n--)
    {
        Rover rover = roverList[n];
        rover.UpdateRover();
    }
}


We can use Enumerable.Reverse to do the reversing for us. It's not much of a change, but I think it makes it a little easier for the reader to see exactly what is happening:

public void UpdateRovers()
{
    foreach (var rover in Enumerable.Reverse(this.rovers))
    {
        rover.Update();
    }
}

Code Snippets

public List<Rover> roverList = new List<Rover>();
public void DestroyRovers()
{
    for (int n = roverList.Count-1; n >= 0; n--)
    {
        Rover rover = roverList[n];

        if (rover.RoverDead())
        {
            roverList.Remove(rover);
        }
    }
}
public void DestroyRovers()
{
    this.rovers.RemoveAll(rover => rover.RoverDead());
}
public bool RoverDead()
{
    return this.lifeTime > 0;
}
public bool IsDead
{
    get { return this.lifeTime > 0; }
}

Context

StackExchange Code Review Q#86480, answer score: 14

Revisions (0)

No revisions yet.