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

Simple Text Adventure: Cleaning up after the party

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

Problem

This week's Code Review Weekend Challenge is about implementing a simple console game.

Here's my game loop:

class Program
{
    static void Main(string[] args)
    {
        var player = new Player();
        var intro = new IntroScreen(player);
        var nextScreen = intro.Run();

        while (nextScreen != null)
        {
            nextScreen = nextScreen.Run();
        }
    }
}


So I have a Player class:

public class Player
{
    public string Name { get; set; }
    public ObservableCollection Inventory { get; private set; }

    public Player()
    {
        Inventory = new ObservableCollection();
        Inventory.CollectionChanged += Inventory_CollectionChanged;
    }

    private void Inventory_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        if (e.Action != NotifyCollectionChangedAction.Add) return;
        foreach (InventoryItem item in e.NewItems)
        {
            Console.WriteLine("Received '{0}'!\n({1})", item.Name, item.Description);
        }

        Console.WriteLine();
        Console.ReadLine();
    }

    public bool HasItem(string name)
    {
        return Inventory.Any(item => item.Name.ToLower() == name.ToLower());
    }
}


The player has an Inventory that's made up of InventoryItem instances:

public struct InventoryItem
{
    public string Name { get; set; }
    public string Description { get; set; }
}


So the role of the Player is really to carry an inventory, whatever that is; whenever an item is added to the player's inventory, there's an output to the console.

The game's mechanics are tucked inside a GameScreen abstract class:

```
public abstract class GameScreen
{
protected IDictionary> MenuItems;

public abstract GameScreen Run();

protected void Write(string text)
{
Console.Write(text);
Console.WriteLine();
Console.WriteLine("[ENTER]");
Console.ReadLine();
}

protected string Prompt(string text)

Solution

Well, i am new to this weekend-challenge thing so I am not sure, from which stand point should i review such question. I'll give it a try.

-
I like your game loop. Its simple and makes sense. It can be simlified further though:

var activeScreen = new IntroScreen(player);
while (activeScreen != null) // do-while, if you are not a hater ;)
{
    activeScreen = activeScreen.Run();
}


-
I don't like your intro screen. It feels like it consists of two separate screens (1 - where you try to recall your name, 2 where you clean up). As it is - it is somewhat difficult to follow the execution path.

-
I think instead of being a protected method Menu should be an actual screen class which accepts options as constructor parameter. That would simplify reading.

-
I dont like the use of an observable collection, i think it is an overkill. And it exposes the methods it should not. A simple wrapper around List which only exposes AddItem and HasItem methods would be much batter. At very least - collection should not be public.

-
Player should probably be a protected property of your base screen.

-
return Inventory.Any(item => item.Name.ToLower() == name.ToLower()); I think ToLower() is confusing. Imho you should not manipulate properties which you use as an ID. Its better to make sure that all the green bags are called "GREEN BAG" instead (by saving it to constant field, for example)

-
In general, i do not like implementation which uses hardcoded strings. I think that proper implementation should use scripts or (if the implementation should be C#-only) XML parsing. But i guess its a bit too much work for a two-hour challenge, right? :)

Code Snippets

var activeScreen = new IntroScreen(player);
while (activeScreen != null) // do-while, if you are not a hater ;)
{
    activeScreen = activeScreen.Run();
}

Context

StackExchange Code Review Q#38203, answer score: 5

Revisions (0)

No revisions yet.