patterncsharpMinor
RPSLS refactored to Object Oriented
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
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()
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
Original Code:
New Code:
I am pretty sure that you can use
I have been wrong before though
Even Newer Code
Here you have kept the same functionality and reduced the code to one loop.
I would get rid of your
playAgain variable and replace the while statement like thisOriginal 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.