RPG character creation implementation

This is a little yet unfinished RPG I made with C#. It runs well but I'm wondering if there are things I could improve to make it more clear or optimal. I'm basically asking for some feedback and suggestions.

The program is obviously not finished but I'm asking now before I move on so I know if I got some serious design or logic issues there. I'm a beginner in programming so keep that in mind when answering.

```
class Main
{
static void Main(string[] args)
{
Console.ForegroundColor = ConsoleColor.White;
Console.WriteLine("1. New Game\n" +
"2. Load Game\n" +
"3. Exit Game\n");
string choice = Console.ReadLine();
bool correctChoice = false;
Hero newHero = new Hero();
while (correctChoice == false)
{
switch (choice)
{
case "1":

CharacterCreation.createChar(newHero);
correctChoice = true;
break;
case "2":
Console.WriteLine("This feature is not yet implemented.");
correctChoice = false;
break;
case "3":
Environment.Exit(0);
break;
default:
Console.WriteLine("Please enter a correct value.");
break;
}
if (correctChoice == true)
break;
else
choice = Console.ReadLine();
}
Console.WriteLine("\n Your character is {0}, a {1}.", newHero.heroName, newHero.heroClass);

Console.ReadLine();
}
}

class CharacterCreation
{
public static void createChar(Hero newHero)
{
Console.WriteLine("\n We will proceed in character creation.");
Console.WriteLine("Please enter your character name:");
newHero.heroName = Console.ReadLine();

Console.WriteLine("\nNow c

I haven't done much work in C#, but from a general coding perspective...

class Main
{
    static void Main(string[] args)
    {
        // lotsa stuff....
    }
}

These (the class and method both) really ought to be public, as I believe that most application runners/executables will be looking for public methods. Additionally, a Main() method should be mostly empty, containing just enough code to launch the rest of the application (whether this submits an additional process or not).

Console.WriteLine()  // Many times

Ideally, never call out to the console specifically. What you should be doing is injecting some sort of output/input stream. It's fine if these actually come from the console, but explicitly tying it to the console itself is frowned upon.

Console.WriteLine("1. New Game\n" +
                  "2. Load Game\n" +
                  "3. Exit Game\n");

So... you're using WriteLine() to write multiple lines at a time? Seems a little counter-intuitive. I'm slightly torn between recommending using individual calls (and potentially having output re-ordered because of some threading issue), or just using Write()...

while (correctChoice == false)
{
    // stuff
}

Using booleans like this is frowned upon; don't compare the boolean with a value, use the boolean itself (that's what it's for) - so, it should be while (!correctChoice) { ... }. Additionally, the name is somewhat ambiguous: possibly availableOptionChosen?

switch (choice)
{
    // lotsa stuff
}

Look into something called the Strategy Pattern. While the use of a switch works out for simple things, software patterns are something to always keep in mind.

case "2":
    Console.WriteLine("This feature is not yet implemented.");
    correctChoice = false;
    break;

Strictly speaking, correctChoice = false; shouldn't be necessary.

case "3":
    Environment.Exit(0);
    break;

Using Environment.Exic(0); should end the running thread. However (if this is anything like Java's System.Exit()) this may have unintended consequences. It would be much better to simply return to the next level.

default:
    Console.WriteLine("Please enter a correct value.");
    break;

You check the 'remaining' cases. Good; generally speaking, you should always do so, even if it's impossible (some of those should maybe only be 'asserts', used for verification-time checks).

Console.ReadLine();
    }
}

You read a line immediately before the program exits. Why?

class CharacterCreation
{

     // Lotsa stuff
}

This is semantically a Builder-pattern, so the class name is a little odd - perhaps CharacterCreator?

public static void createChar(Hero newHero) 
{
    // Stuff
}

Why the abbreviation of 'Character' (and why all the abbreviations, generally)? Also, if it's named 'create...', you shouldn't be passing it in - it should be the return type (alternatively, call it setupCharacter(), and pass in the Hero). Additionally, newHero is an awkward name here - just use hero (you don't have an oldHero you're using, do you?).

while (correctChoice == false)
{
    switch (classChoice)
    {

The stuff I mentioned about your main loop applies here as well. This is especially prevalent with the class, if you're eventually going for something like DnD (which can have hundreds of classes).

class Character
{
    public int charHealth, charMaxHealth, charStr, charInt, charAgi;
}

Always put member definitions on their own lines. Always make them private, and provide equivalent public properties. Don't abbreviate unnecessarily, seek to be understandable. Don't prefix variable names with their enclosing class' name (especially abbreviated) - it's noise.

class Hero : Character
{
     // Acceptable
}

This works, although it may become awkward in the future.

public static void ClassSelect(Hero newHero)
{
    newHero.charAgi = 10;
    newHero.charInt = 10;
    newHero.charMaxHealth = 100;
    newHero.experience = 0;
    newHero.gold = 0;
    newHero.level = 1;

    switch (newHero.heroClass)
    {
        case "Warrior":
            newHero.charMaxHealth = newHero.charMaxHealth * 115 / 100;
            newHero.charStr = newHero.charStr * 12 / 100;
            break;
        case "Mage":
            newHero.charInt = newHero.charInt * 12 / 100;
            break;
        case "Rogue":
            newHero.charAgi = newHero.charAgi * 12 / 100;
            break;
    }
    newHero.charHealth = newHero.charMaxHealth;
}

This, though, belongs in CharacterCreation` (that's what it's there for). Additionally, instead of

newHero.charXxx = newHero.charXxx * 12 / 100;

you should probably just be setting it to the desired value. Especially because that's probably a bug, since a warrior will start out with a higher Int (10) than a mage

((10 * 12) / 100) = 120 / 100 = 1.2 -> 1

which I doubt is what you want

StackExchange Code Review Q#20102, answer score: 12