This is sophisticated for a first attempt, but today would be a great day to break yourself of bad habits. Your code is full of them.
Start with: format your code using standard formatting conventions. We understand code more easily if it is vertical, and your code is horizontal.
Let's go through this line by line.
static void Main(string[] args)
{
Programs that do everything in Main are programs that cannot be easily refactored or re-used. Main should create an object that represents the program and then execute it.
Random GenRandom = new Random();
Follow C# naming conventions. Locals get camelCase names. This should be
Random random = new Random();
int t = 0, r, c1 = random.Next(1, 6), c2 = random.Next(1, 6), c3 = random.Next(1, 6), c4 = random.Next(1, 6);
Though this style is legal, it's considered poor style. Declare and initialize variables one per line. Give your variables meaningful names. You will not die younger if you spend time typing turn instead of t. This should be:
int turn = 0;
int digit;
int cell1 = random.Next(1, 6);
int cell2 = random.Next(1, 6);
int cell3 = random.Next(1, 6);
int cell4 = random.Next(1, 6);
Moving on:
bool w = false;
Same deal:
bool hasWon = false;
Moving on:
string Input, Code = Convert.ToString(c1);
Code += c2;
Code += c3;
Code += c4;
Again, one per line.
We now see that we have the same data in two places: the locals c1, c2, c3, c4 which are never used again and Code. It is time for our first refactoring. What we want is:
string code = GenerateCode();
Let's implement that:
static Random random = new Random();
static string GenerateCode()
{
return random.Next(1, 6).ToString() +
random.Next(1, 6).ToString()
random.Next(1, 6).ToString()
random.Next(1, 6).ToString();
}
And we remove random, c1, and so on, from our method.
All right, where were we? We now have:
string input;
string code = GenerateCode();
Moving on:
while (t != 8) {
First, bad form in the disco brace style. Second, eight? Where did eight come from? Rewrite it:
const int maximumTurns = 8;
while (turn != maximumTurns)
{
Moving on:
t++;
I find increment operators distasteful.
turn += 1;
Much nicer.
Unepic:
OH THE PAIN. Do not write unstructured code in C#; it is 2018, not 1972.
Console.Clear();
The first line I don't object to! :-) But as we'll see, it should be in a method of its own.
Console.WriteLine("You have {0} turn(s) left.",9-t);
Better would be:
Console.WriteLine($"You have {maximumTurns + 1 - turn} turn(s) left.");
Because now if you change maximumTurns to 7, you only have to change the code in one place, not many.
Better still, make a helper function:
static string Plural(int x, string s)
{
return x == 1 ? s : s + "s";
}
and then
int remainingTurns = maximumTurns - 1 - turn;
Console.WriteLine($"You have {remainingTurns} {Plural(remainingTurns, "turn")} left.");
And then move that into a helper.
Moving on:
Console.WriteLine("Guess the code E.g 1561: ");
Eschew Latinisms. Should be:
Console.WriteLine("Guess the code (for example, 1561): ");
Now we come into a section that demands refactoring:
input = Console.ReadLine();
Fine so far...
if (input.Length != 4) goto Unepic; // Checks if input is 4 characters long
Do not write comments that repeat what is plainly evident in the code. And do not use spaghetti logic.
try { Convert.ToInt16(Input); Convert.ToString(Input); } catch (FormatException) { goto Unepic; } // Checks if input is only numbers
Do not re-invent the wheel; TryParse already exists:
int number; // In C# 7 you can use a discard instead.
bool valid = int.TryParse(input, out number);
How should we structure this whole thing better? What we want is:
if (!ValidInput(input)) goto Unepic;
So write that:
static bool ValidateInput(string input)
{
int number;
return input.Length == 4 && int.TryParse(input, out number);
}
Moving on:
if (Input == Code) { w = true; goto End; }; // Checks if you've won
There's a feral semi at the end there. Remove it.
Again, when you are writing comments that are obvious, ask yourself "how could I rewrite the code so that the comment was unnecessary?" Like this:
if (input == code)
{
hasWon = true;
goto End;
}
We'll address the goto later.
if (Input.Contains("0") || Input.Contains("7") || Input.Contains("8") || Input.Contains("9")) { goto Unepic; } // Checks if it has any digits that are 0 or above 7
Why is this check here? It should be in ValidateInput. Put it there!
We then have a little loop:
r = -1;
while (r != 3)
{
r++;
if (Input[r] == Code[r]) Console.Write(1); else Console.Write(0); // Checks if a digit of the input is equal to a digit of the code
}
Again, put it in a method.
static void WriteHints(string input, string code)
{
The normal way we would write this loop is:
for (int i = 0; i < input.Length; i += 1)
Console.Write(input[i] == code[i] ? 1 : 0;
}
Finishing up the game loop:
Console.WriteLine();
Console.Write("Press any key to continue.");
Console.ReadKey(true);
Why make the user press a key here? But again, it could be put into a helper method.
}
End:;
Console.Clear();
Again, I'm fine with this but we'll come back to the goto later.
if (w == true) { Console.WriteLine("You won! The code you guessed was {0}.", Code); } else { Console.WriteLine("You lost! The code you couldnt guess was {0}.",Code); };
Writing if (w == true) is a sure sign of newbie code. w is already either true or false; you don't have to say "if it is true that w is true" and you don't have to say "if w is true" -- just say "if w".
And again, this could go into a helper.
Now, let's look at your main loop as I have refactored it, all together
static void Main(string[] args)
{
int turn = 0;
bool hasWon = false;
string code = GenerateCode();
string input;
const int maximumTurns = 8;
while (turn != maximumTurns)
{
turn += 1;
Unepic:
PromptUserForInput(maximumTurns, turn);
input = Console.ReadLine();
if (!ValidInput(input)) goto Unepic;
if (input == code)
{
hasWon = true;
goto End;
}
WriteHints(input, code);
PromptUserToPressKey();
}
End:;
PrintWinOrLoseMessage(hasWon, code);
}
This is already one million times easier to read but we are not done.
The first thing we notice is that we do some work to ensure that turn is not incremented if the user did a bad input. That's great! But the code does not read like that intent. Let's rewrite it so it does.
int turn = 1; // not zero!
while (turn <= maximumTurns) // not !=
{
Unepic:
PromptUserForInput(maximumTurns, turn);
input = Console.ReadLine();
if (!ValidInput(input)) goto Unepic;
if (input == code)
{
hasWon = true;
goto End;
}
WriteHints(input, code);
PromptUserToPressKey();
turn += 1;
}
Now turn is only ever incremented after the user has put in a good guess, and the code doesn't need to initialize turn to the wrong value so that it can be incremented later.
Now we notice that the goto Unepic and goto End are continue and break:
while (turn <= maximumTurns)
{
PromptUserForInput(maximumTurns, turn);
input = Console.ReadLine();
if (!ValidInput(input))
continue;
if (input == code)
{
hasWon = true;
break;
}
WriteHints(input, code);
PromptUserToPressKey();
turn += 1;
}
Look at how much easier to understand my version is compared to yours. Anyone, even a non-coder, could look at this thing and just read it like English. "while the turn is less than or equal to the maximum turns, prompt the user for input, then read the line from the console, then validate the input..."
That is what you must strive for in your code. Always be asking yourself how could I make this easier to understand? How could I make this read more like a description of my intentions?
Could we do better? Sure. We actually have two loops written as one. There's the loop that gets valid user input, and there's the loop that runs the game. Make that explicit:
while (turn <= maximumTurns)
{
do
{
PromptUserForInput(maximumTurns, turn);
input = Console.ReadLine();
}
while(!ValidInput(input));
if (input == code)
{
hasWon = true;
break;
}
WriteHints(input, code);
PromptUserToPressKey();
turn += 1;
}
And now we see another opportunity for helper methods. Move the inner loop to a helper! Move the winning check to a helper:
while (turn <= maximumTurns)
{
input = ObtainValidatedInput(maximumTurns, turn);
hasWon = CheckForWin(input, code);
if (hasWon)
break;
WriteHints(input, code);
PromptUserToPressKey();
turn += 1;
}
Were I writing your code from scratch, I would make it considerably more abstract than even the version I've given you here. My main would be:
static void Main()
{
var game = new Game();
game.Start();
while (!game.IsOver)
game.ExecuteTurn();
game.End();
}
Try writing your code starting from this template. Notice how clear and logical it forces your code to be when you start from a position of the code reading like an abstract description of the workflow. Make every method like this, where you can read the logical workflow right out of a half dozen lines of code. Get in good habits while you are still a beginner.
