Return to Revisions

1. An enum will serve you much better than a list of strings.

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

2. A naming convention. If a variable is a counter and its name describes what is being counted, it should then be plural.

    int wins = 0;
    int loses = 0;
    int ties = 0;
   

3. And now at this point, I don't want to just correct small bits of this code, but restructure it to use some OOP. But I won't, just know that next time you make a game, OOP is much better, modularization is the keyword right there (if it even is a word).

4. The Method Name gameSetup should start with a capital letter. Especially because it is public, if it was private you could make it lower-case, though I would still not recommend it.

5. Inside GameSetup you really aren't doing a lot of logic that you would normally want removed from the pretty business logic. On top of that, you are only calling this function in one place in your code, so it benefits you about.... not at all. And now that I've taken away your gesture list I will replace the code that you will need here for that.

   Using foreach:

    foreach (Gesture gesture in (Gesture[])Enum.GetValues(typeof(Gesture)))
    {
        Console.WriteLine((int)gesture + Enum.GetName(typeof(Gesture), gesture));
    }
   

   Using for:

    for (int i = 1; i <= 5; i++)
    {
        Console.WriteLine(i + Enum.GetName(typeof(Gesture), i));
    }
   

   While either of the above loops will work, I would recommend only using the first one. As the second one only works aslong as you don't change the enum at all, which in any other situation would not work out so well.

6. You mentioned that you weren't sure about how you were using the Random class. You said or if Random should be a static variable in the class scope. Yes, you should move rand to the class scope. However that is not 100% necessary. I want to say though, that as you are declaring rand in a static method, it is a static variable.. infact everything you are doing here is static. If you had gone the way of OOP you would have had some non-static stuff, but.. you didn't.

7. ComputerGesture is another example of a method which is only called once and does very little code. This would have been better suited to stay with the rest of the main logic and just had a comment placed above it saying, computer gesture. Also a code change, which is just something I like to do.. replace the contents of that method with.

    return options[new Random().Next(0, options.Count)];
   

   however, because I changed your string array to enums, this is what it would have to look like

    public static Gesture ComputerGesture()
    {
        var vals = Enum.GetValues(typeof(Gesture));
        return (Gesture)vals.GetValue(new Random().Next(vals.Length);
    }
   

.... And I must leave right now. So stay tuned for more.