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

RPSLS refactored to Object Oriented

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

Problem

I wrote a couple reviews for this CR post. In my most recent review, I refactored @Malachi 's code to fit OO design. I'm looking for any advice/hints/criticisms on it.

A review is welcome for both the OO design I implemented and the main() method which is a bit sloppy.

Here is the entire dump:

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

namespace RPSLS
{
class Program
{
static void Main(string[] args)
{
var endGameMenu = new string[] { "Play Again", "Clear Score", "Quit" };
var me = new Human();
var computer = new Computer();
var playAgain = true;

do
{
Game.Play(me, computer);
Console.WriteLine("Your scorecard: " + me.GetScoreCard());
int result;
do
{
Console.WriteLine("Options:");
Utils.PrintMenu(endGameMenu.ToList());
result = Utils.PromptForRangedInt(0, endGameMenu.Length - 1, "Choose an Option: ");
if (result == 1)
{
me.ClearScore();
Console.Clear();
Utils.WriteLineColored("Your score has been cleared", ConsoleColor.Green);
}
} while (result != 0 && result != 2);
Console.Clear();
playAgain = result == 0;
} while (playAgain);
}
}

enum Gesture
{
Rock = 1,
Paper = 2,
Scissors = 3,
Spock = 4,
Lizard = 5
}

enum Performance
{
Lost = -1,
Tied = 0,
Won = 1
}

abstract class Player
{
public uint Wins { get; private set; }
public uint Loses { get; private set; }
public uint Ties { get; private set; }

public abstract Gesture GetMove();

public string GetScoreCard()

Solution

one thing to start with.

I would get rid of your playAgain variable and replace the while statement like this

Original Code:

do
{
    Game.Play(me, computer);
    Console.WriteLine("Your scorecard: " + me.GetScoreCard());
    int result;
    do
    {
        Console.WriteLine("Options:");
        Utils.PrintMenu(endGameMenu.ToList());
        result = Utils.PromptForRangedInt(0, endGameMenu.Length - 1, "Choose an Option: ");
        if (result == 1)
        {
            me.ClearScore();
            Console.Clear();
            Utils.WriteLineColored("Your score has been cleared", ConsoleColor.Green);
        }
    } while (result != 0 && result != 2);
    Console.Clear();
    playAgain = result == 0;
} while (playAgain);


New Code:

do
{
    Game.Play(me, computer);
    Console.WriteLine("Your scorecard: " + me.GetScoreCard());
    int result;
    do
    {
        Console.WriteLine("Options:");
        Utils.PrintMenu(endGameMenu.ToList());
        result = Utils.PromptForRangedInt(0, endGameMenu.Length - 1, "Choose an Option: ");
        if (result == 1)
        {
            me.ClearScore();
            Console.Clear();
            Utils.WriteLineColored("Your score has been cleared", ConsoleColor.Green);
        }
    } while (result != 0 && result != 2);
    Console.Clear();
} while (result == 0);


I am pretty sure that you can use result because anything inside the loop is within the same scope as what is in the while clause as long as it is part of a do while statement.

I have been wrong before though

Even Newer Code

int result;
do
{
    Game.Play(me, computer);
    Console.WriteLine("Your scorecard: " + me.GetScoreCard());
    Console.WriteLine("Options:");
    Utils.PrintMenu(endGameMenu.ToList());
    result = Utils.PromptForRangedInt(0, endGameMenu.Length - 1, "Choose an Option: ");
    if (result == 1)
    {
        me.ClearScore();
        Console.Clear();
        Utils.WriteLineColored("Your score has been cleared", ConsoleColor.Green);
    }
    Console.Clear();
} while (result == 0 || result == 1);


Here you have kept the same functionality and reduced the code to one loop.

Code Snippets

do
{
    Game.Play(me, computer);
    Console.WriteLine("Your scorecard: " + me.GetScoreCard());
    int result;
    do
    {
        Console.WriteLine("Options:");
        Utils.PrintMenu(endGameMenu.ToList());
        result = Utils.PromptForRangedInt(0, endGameMenu.Length - 1, "Choose an Option: ");
        if (result == 1)
        {
            me.ClearScore();
            Console.Clear();
            Utils.WriteLineColored("Your score has been cleared", ConsoleColor.Green);
        }
    } while (result != 0 && result != 2);
    Console.Clear();
    playAgain = result == 0;
} while (playAgain);
do
{
    Game.Play(me, computer);
    Console.WriteLine("Your scorecard: " + me.GetScoreCard());
    int result;
    do
    {
        Console.WriteLine("Options:");
        Utils.PrintMenu(endGameMenu.ToList());
        result = Utils.PromptForRangedInt(0, endGameMenu.Length - 1, "Choose an Option: ");
        if (result == 1)
        {
            me.ClearScore();
            Console.Clear();
            Utils.WriteLineColored("Your score has been cleared", ConsoleColor.Green);
        }
    } while (result != 0 && result != 2);
    Console.Clear();
} while (result == 0);
int result;
do
{
    Game.Play(me, computer);
    Console.WriteLine("Your scorecard: " + me.GetScoreCard());
    Console.WriteLine("Options:");
    Utils.PrintMenu(endGameMenu.ToList());
    result = Utils.PromptForRangedInt(0, endGameMenu.Length - 1, "Choose an Option: ");
    if (result == 1)
    {
        me.ClearScore();
        Console.Clear();
        Utils.WriteLineColored("Your score has been cleared", ConsoleColor.Green);
    }
    Console.Clear();
} while (result == 0 || result == 1);

Context

StackExchange Code Review Q#44177, answer score: 4

Revisions (0)

No revisions yet.