Writing multiple if tags that compare two variables

I am a beginner in C# coding, and I was trying to compare two int variables:

void CompareNumber() {
    int oneNumber;
    int secondNumber;

    if (oneNumber > secondNumber)
        {
        DoSomething();
        }

    else if (oneNumber < secondnumber)
        {
        DoSomethingElse();
        }

    else if (oneNumber == secondnumber)
         {
         DoSomethingDifferent();
         }
}

While this does work, it looks kinda messy, especially because I compare variables in this manner many times. Is there a more concise way of doing this, making it look neater? (Except for omitting the curly brackets)

Here is my actual code for a simple game that thinks of a number, and you need to guess what it said:

```
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace GuessName
{
class Program
{
static void Main(string[] args)
{
int previousNumber = 0;
AskNumber(previousNumber);
}

static void AskNumber(int previousNumber)
{
Console.Clear();

int numberTyped;
int randomNumber = new Random().Next(1, 11);

while (previousNumber == randomNumber)
randomNumber = new Random().Next(1, 11);

Console.WriteLine("I am thinking of a number between 1 and 10. What do you think it is?");

if (int.TryParse(Console.ReadLine(), out numberTyped) == true)
CheckNumber(numberTyped, randomNumber);
else
InvalidNumber();
}

static void CheckNumber(int numberTyped, int randomNumber)
{
if (numberTyped 10)
{
InvalidNumber();
}
else if (numberTyped > randomNumber)
{
Console.Clear();
Console.WriteLine("Sorry, but your num

Indentation?

At first I thought it was a mere copy/paste error, but the pattern is throughout the code you submitted, so I have to mention it: it's not the braces you indent, it's what the braces encompass!

static void Main(string[] args)
        {
        int previousNumber = 0;
        AskNumber(previousNumber);
        }

Should be:

static void Main(string[] args)
    {
        int previousNumber = 0;
        AskNumber(previousNumber);
    }

Curly braces define a scope. In C#, convention is to place the scope-opening brace on the next line, so this:

void CompareNumber() {

Should be:

void CompareNumber() 
{

Speaking of braces:

Is there a more concise way of doing this, making it look neater? (Except for omitting the curly brackets)

Omitting the curly braces does not make code look neater. Proof being this very confusing snippet:

if (wantToPlay == "y")
            AskNumber(0);
        else
            Environment.Exit(0);
        }

I had to scan the entire method twice to figure out that the last } was in fact closing the scope of the method.

By opposition:

if (wantToPlay == "y")
        {
            AskNumber(0);
        }
        else
        {
            Environment.Exit(0);
        }
    }

Is brutally in-your-face crystal-clear. Notice the position of the last brace, which closes the method scope. The readability problem stems from the extraneous indentation of scope-delimiting braces - this doesn't suffer the exact same issue:

if (wantToPlay == "y")
            AskNumber(0);
        else
            Environment.Exit(0);
    }

You should always delimit scopes with braces. if/else statements denote a scope - that scope wants its braces.

Same with loop constructs:

while (previousNumber == randomNumber)
            randomNumber = new Random().Next(1, 11);

Should be:

while (previousNumber == randomNumber)
        {
            randomNumber = new Random().Next(1, 11);
        }

The Good

You use descriptive naming. This is often understated - descriptive names are your best friend. They reduce bug-proneness of your code all by themselves, and make code easier and more enjoyable to read. Since programming is 80% reading and 20% writing, enjoyable reading means enjoyable programming.

Good job!

The Bad

The syntax for an if condition goes if ([bool-expression]), where [bool-expression] is any expression that evaluates to true or false. Can you spot the redundancy here?

if (int.TryParse(Console.ReadLine(), out numberTyped) == true)

That's right. TryParse returns a bool that's a perfectly valid [bool-expression] - in fact, anytime you're comparing a Boolean value (or expression) to a Boolean constant for the sake of getting a Boolean expression, you are needlessly repeating yourself... redundantly.

Just do this instead:

if (int.TryParse(Console.ReadLine(), out numberTyped))

Method names should start with a verb. Always. void InvalidNumber() is a bad name, because it doesn't say anything about what it actually does - it doesn't return a number, and doesn't intake one either. Without looking at how it's implemented, it's pretty confusing.

Console applications exit when the Main method executes to its closing brace. I have never used Environment.Exit(0) in any application I've written - in fact, this call is like a "red button", a forceful, ugly way to kill your application (and throw the body in a container in the back alley) rather than letting it gracefully exit through the front door.

You're going to have to restructure things up in order to make that happen though.

CheckNumber shouldn't have the 0 and 10 hard-coded like this; there should be constants defined for the lower and upper bounds, and the AskNumber method should be using them as well - that way if you ever need to change the 10 to a 100, there's only 1 place you'll need to change it.

AskNumber shouldn't be creating a new Random() every time. There should be one single instance of Random that the program uses whenever it needs a random number.

There's a lot more to say about this code, I'll let other reviewers chip in ;)

StackExchange Code Review Q#77038, answer score: 12