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

RPSLS game revisited. Is my design better than previous attempts?

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

Problem

This question is a follow up to these questions:

-
RPSLS Game in C#

-
Ensuring user input is an integer in a range

-
RPSLS is less messy now, but is it clean?

Right now I have a couple of ideas in the think tank about how I want to code what I like to call "killMessage" the whole process seems a little intimidating at the moment, in other words I haven't decided exactly how it is going to work out yet.

There is plenty of code that I borrowed/altered/stole from other answers, but I think we all assumed that was going to happen a little bit, and I think that I understand most of what I included, so I still learned something from it and didn't just copy pasta in the code.

I really like the idea of a Gesture class so that I can actually give the Gesture abilities.

```
///
/// Thanks to
/// https://codereview.stackexchange.com/users/38054/benvlodgi
/// https://codereview.stackexchange.com/users/4318/eric-lippert
/// https://codereview.stackexchange.com/users/23788/mats-mug
/// https://codereview.stackexchange.com/users/30346/chriswue
///
namespace RPSLSGame
{
class MainClass
{
public static void Main (string[] args)
{
/* Here are your rules:
"Scissors cuts paper,
paper covers rock,
rock crushes lizard,
lizard poisons Spock,
Spock smashes scissors,
scissors decapitate lizard,
lizard eats paper,
paper disproves Spock,
Spock vaporizes rock.
And as it always has, rock crushes scissors."
-- Dr. Sheldon Cooper */
int win = 0;
int lose = 0;
int tie = 0;

IList gameGestures = GetGestures();

do{
Console.WriteLine("Please choose your Gesture ");
PrintMenu(gameGestures);
var playerGesture = gameGestures[PromptForNumber("Please choose your gesture",1,gameGestures.Count)-1];

Solution

I'm going to leave a broader review to other answers, and only focus on a single line of code:

throw new Exception("Gesture is sealed");


Don't do that.

There are really two alternatives to throwing exceptions.

One is to throw an existing exception, derived from System.Exception. This is trickier than it sounds, because you need to pick your exception carefully - you want to throw a meaningful exception.

In this case an InvalidOperationException seems in line with the Principle of Least Astonishment:

InvalidOperationException Class


The exception that is thrown when a method call is invalid for the object's current state.
http://msdn.microsoft.com/en-us/library/system.invalidoperationexception(v=vs.110).aspx

There are a few exception subclasses like this one (and ArgumentException and its derivatives), that are very often the most meaningful and written-for-that-purpose exception to use.

Another way would be to define your own. That's easier that it sounds, you just derive a new class from System.Exception:

public class GestureSealedException : Exception
{
    public GestureSealedException()
        : base("Gesture is sealed.") { }
}


And now you can throw and catch a GestureSealedException.

That said...


Don't throw System.Exception.

In my quick research for why that class wasn't abstract in the first place if it wasn't intended to be instanciated and thrown, I found an interesting discussion on Programmers.SE. Emphasis mine:


when coding small demo apps or proof-of-concepts, I don't want to start designing 10 different exception sub-classes, or spending time trying to decide which is the "best" exception class for the situation. I'd rather just throw Exception and pass a string that explains the details. When it's throw-away code, I don't care about these things [...]


https://softwareengineering.stackexchange.com/a/119683/68834

The flipside of this out-of-context quote, is that when it is quality code, you do care about these things.

Code Snippets

throw new Exception("Gesture is sealed");
public class GestureSealedException : Exception
{
    public GestureSealedException()
        : base("Gesture is sealed.") { }
}

Context

StackExchange Code Review Q#44487, answer score: 11

Revisions (0)

No revisions yet.