1
\$\begingroup\$

I just started coding c# 4 days ago. I just finsihed my first ever Console Program. I would like to hear some suggestions, on how I could improve my code. I would also like to add that this is my first programming language, so I'm still very new to coding.

Also, the namespace says FourFunctionCalcProgram, but the calculator can dore more than 4 things. I just don't know how to change it.

using System;
using System.Threading.Tasks;
using System.Threading;
using System.Linq.Expressions;
using System.Diagnostics;
using System.Runtime.InteropServices.ComTypes;

namespace FourFunctionCalcProgram
{
class Program
{
    static void Main(string[] args)
    {
        int Age = 0;
        string Answer = "Null";
        string Answer2 = "Null";
        bool NeverFalse = true;
        double num = 0;
        double num1 = 0;
        double num2 = 0;
        string fun = "";


        Console.BackgroundColor = ConsoleColor.White;
        Console.ForegroundColor = ConsoleColor.Black;
        Console.Clear();

        Console.Write("Hello, welcome to this calculator.\nBefore we can begin, I would like to ask you for some basic information.");
        Thread.Sleep(5000);
        Console.Clear();
        Console.Write("First of all, please enter your name here: ");
        string Name = Console.ReadLine();

        do
        {
            try
            {
                Console.Clear();
                Console.Write("Ok, hello " + Name + " \nPlease enter your age here: ");
                Age = Convert.ToInt32(Console.ReadLine());
            }
            catch (FormatException e)
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine(e.Message);
                Console.WriteLine("Please Try Again");
                Console.ForegroundColor = ConsoleColor.Black;
                Thread.Sleep(1000);
                Console.Clear();
            }
        } while (Age == 0);

        do
        {

            Console.Clear();
            Console.Write("Ok, so your name is " + Name + " And you are " + Age + " Years old? ");
            Answer = Console.ReadLine();


            if (Answer == "Yes" || Answer == "yes" || Answer == "Yep" || Answer == "yep" || Answer == "Correct" || Answer == "correct" || Answer == "True" || Answer == "true")
            {
                Console.WriteLine("That's great to hear!");
                Thread.Sleep(2000);
                break;
            }

            if (Answer == "No" || Answer == "no" || Answer == "Nope" || Answer == "nope" || Answer == "False" || Answer == "false" || Answer == "Incorrect" || Answer == "incorrect")
            {
                Console.Write("I'm sorry to hear that, what is incorrect? ");
                Answer2 = Console.ReadLine();

                if (Answer2 == "Name" || Answer2 == "name" || Answer2 == "My name" || Answer2 == "my name" || Answer2 == "My Name" || Answer2 == "The name" || Answer2 == "The Name" || Answer2 == "the name")
                {
                    Console.Clear();
                    Console.Write("What is your real name? ");
                    Name = Console.ReadLine();
                }

                else if (Answer2 == "Age" || Answer2 == "age" || Answer2 == "My age" || Answer2 == "my age" || Answer2 == "My Age" || Answer2 == "The age" || Answer2 == "The Age" || Answer2 == "the age")
                {

                    try
                    {
                        Console.Clear();
                        Console.Write("What is your real age? ");
                        Age = Convert.ToInt32(Console.ReadLine());
                    }
                    catch (FormatException e)
                    {
                        Console.ForegroundColor = ConsoleColor.Red;
                        Console.WriteLine(e.Message);
                        Console.WriteLine("Please try again");
                        Thread.Sleep(1000);
                        Console.Clear();
                        Console.ForegroundColor = ConsoleColor.Black;
                    }
                }
            }
            else
            {
                Console.ForegroundColor = ConsoleColor.Red;
                Console.WriteLine("I'm sorry, but I don't understand what you mean by " + Answer + ". Please try again");
                Thread.Sleep(1000);
                Console.ForegroundColor = ConsoleColor.Black;
            }

        } while (NeverFalse);



        Console.ForegroundColor = ConsoleColor.White;
        Console.BackgroundColor = ConsoleColor.Black;
        Console.Clear();
        for (int i = 10; i <= 100; i += 10)
        {
            Console.WriteLine("Calculating \n" + i + "%");
            Thread.Sleep(500);
            Console.Clear();
        }
        Console.BackgroundColor = ConsoleColor.White;
        Console.ForegroundColor = ConsoleColor.Black;
        Console.Clear();

        if (Age <= 18)
        {
            Console.WriteLine("Sorry, but your are too young to utilise this program. Please come back in ", 18 - Age, " years.");
        }
        else
        {
            Console.WriteLine("Welcome to my calculator.\n Please read the README, for all functions");
            while (NeverFalse)
            {
                Console.Write("Which calculator do you want to use? The 1Number or 2Number? ");
                Answer = Console.ReadLine();
                if (Answer == "1Number")
                {
                    while (NeverFalse)
                    {
                        do
                        {


                            Console.Clear();
                            Console.Write("Please enter your number here: ");

                            try
                            {
                                num = Convert.ToDouble(Console.ReadLine());
                            }

                            catch (Exception e)
                            {
                                Console.ForegroundColor = ConsoleColor.Red;
                                Console.WriteLine(e.Message);
                                Console.WriteLine("Please try again");
                                Console.ForegroundColor = ConsoleColor.Black;
                                Thread.Sleep(1000);
                            }
                        } while (num == 0);

                        Console.Clear();
                        Console.Write("Now, please enter the function: ");
                        Answer = Console.ReadLine();
                        Console.Clear();

                        switch (Answer)
                        {
                            case "ROUND":
                                Console.WriteLine("= " + GetRound(num));

                                Thread.Sleep(1000);
                                Console.Write("Press any key to continue");
                                Console.ReadKey();
                                break;

                            case "SQRT":
                                Console.WriteLine("= " + GetSquareRoot(num));

                                Thread.Sleep(1000);
                                Console.Write("Press any key to continue");
                                Console.ReadKey();
                                break;

                            default:
                                Console.WriteLine("I don't know what you mean by" + Answer + ". Please try again");
                                break;
                        }

                    }



                }
                else if (Answer == "2Number")
                {
                    do
                    {
                        Console.Clear();
                        try
                        {
                            Console.Write("Please enter your first number: ");
                            num1 = Convert.ToDouble(Console.ReadLine());
                            Console.Write("Please enter the function: ");
                            fun = Console.ReadLine();
                            Console.Write("Please enter a second number: ");
                            num2 = Convert.ToDouble(Console.ReadLine());
                        }

                        catch (Exception e)
                        {
                            Console.Clear();
                            Console.ForegroundColor = ConsoleColor.Red;
                            Console.WriteLine(e.Message);
                            Console.WriteLine("Please try again.");
                            Console.ForegroundColor = ConsoleColor.Black;
                            Thread.Sleep(2000);
                            Console.Clear();
                        }

                    } while (num1 == 0 || num2 == 0);
                    switch (fun)
                    {
                        case "+":
                            Console.WriteLine("= " + num1 + num2);
                            Thread.Sleep(1000);
                            Console.Write("Press any key to continue");
                            Console.ReadKey();

                            break;

                        case "-":
                            Console.WriteLine("= ", num1 - num2);
                            Thread.Sleep(1000);
                            Console.Write("Press any key to continue");
                            Console.ReadKey();
                            break;

                        case "/":
                            Console.WriteLine("= " + num1 / num2);
                            Thread.Sleep(1000);
                            Console.Write("Press any key to continue");
                            Console.ReadKey();
                            break;

                        case "*":
                            Console.WriteLine("= " + num1 * num2);
                            Thread.Sleep(1000);
                            Console.Write("Press any key to continue");
                            Console.ReadKey();
                            break;

                        case "%":
                            Console.WriteLine("= " + num1 % num2);
                            Thread.Sleep(1000);
                            Console.Write("Press any key to continue");
                            Console.ReadKey();
                            break;

                        case "MAX":
                            Console.WriteLine("= " + GetMax(num1, num2));
                            Thread.Sleep(1000);
                            Console.Write("Press any key to continue");
                            Console.ReadKey();
                            break;

                        case "MIN":
                            Console.WriteLine("= " + GetMin(num1, num2));
                            Thread.Sleep(1000);
                            Console.Write("Press any key to continue");
                            Console.ReadKey();
                            break;

                        case "POW":
                            Console.WriteLine("= " + GetPow(num1, num2));
                            Thread.Sleep(1000);
                            Console.Write("Press any key to continue");
                            Console.ReadKey();
                            break;                        
                    }
                }
                else
                {
                    Console.WriteLine("Sorry, I do not understand, Please enter '1Number' or '2Number ");
                    Thread.Sleep(1000);
                    Console.Clear();
                }

            }


        }
    }

    static double GetRound(double num)
    {
        num = Math.Round(num);

        return num;
    }

    static double GetSquareRoot(double num)
    {
        num = Math.Sqrt(num);

        return num;
    }
    static double GetMax(double num1, double num2)
    {
        double max = Math.Max(num1, num2);
        return max;
    }
    static double GetMin(double num1, double num2)
    {
        double min = Math.Min(num1, num2);
        return min;
    }


    static double GetPow(double BaseNum, double PowNum)
    {
        double result = 1;

        for (int i = 0; i < PowNum; i++)
        {
            result = result * BaseNum;
        }

        return result;
    }



}

}

\$\endgroup\$
3
  • \$\begingroup\$ Welcome to Code Review. The question about changing the namespace name is off-topic for Code Review, but if you use Visual Studio try selecting what you want to change Edit->Find and Replace and Select Entire Solution. \$\endgroup\$ – pacmaninbw Jul 12 '20 at 13:21
  • \$\begingroup\$ For the difference between "Age" and "age" use the string method .ToLower(). Other than that jump into Classes and start structuring your logic, good first go! \$\endgroup\$ – Philip Pol Jul 12 '20 at 15:24
  • \$\begingroup\$ Your title should state what your code does. codereview.stackexchange.com/help/how-to-ask \$\endgroup\$ – BCdotWEB Jul 12 '20 at 22:03
2
\$\begingroup\$

Good first go at C#, it looks good and seems like it could work.

Some quick observations:

1: When you are listing your variables at the top, instead of doing:

string name = "";
double age = 0;

simply do

string name;
double number;

This allows us to type a bit faster, as for the double, this allows the CLR to not do a cast because remember; a int is 0 while a double would be 0.0

2: I really liked that you used "Convert.ToInt32" this is a safe(r) way of casting because if the to-be casted value was not a number then we would get an exception. ToInt32 (and the like) will always try to cast the value, and if not will default to 0 (amazing). The alternative would be

var myDouble = 5.73;
int myInt = (int)myDouble;

But if mydouble was something else (because of the var it easily could be - which isn't a bad thing and will make more sense in the future) then you will get an exception..

3: When you have user input you'll need to clean it. Get familiar with some of the string's built in methods as it will help you a lot down the road. An example would be

 if (Answer == "Yes" || Answer == "yes" || etc. . . 

before the if we would take the Answer and put a toLower() at the end of it.

string cleanData = Answer.ToLower();
if(Answer == yes ) .. and so on

4: Remember Camel casing and ..whatever the other casing is called. I kept thinking I was trying to do a tolower to a class or method instead of a value or property SO know what needs what casing because it will help you look professional :)

5:

 while (NeverFalse);

could be rewritten as

while(true)

because while cases will always run if true, and if it's just true then it will never ever ever stop working!

..but if for some strange reason you do want to end this forever while loop you can simply do a "break;" and you'll be good. Also won't need to set the NeverFalse variable again so that's always cool

while(true)
{
//we do some really cool stuff

//..but oh wait, I need to do something else now
break;
}

6: I would definitely look into classes now. It will help you break all your code apart and make it a bit more readable!

Other than that, it looks good. You even did a try/catch block!

\$\endgroup\$
1
\$\begingroup\$

A couple (basic) points:

Validation of user input

Exception handling is not the way to go here. Exception handlers should be used to handle unexpected conditions. Since you are handling user input, it is expected that the user may enter invalid values so you should avoid exceptions rather than having to handle them (which is more expensive).

There are different ways of validating user input. To determine if a string is a number you have an example on SO:

var isNumeric = int.TryParse("123", out int n);

(you have many more options)

Handling questions

Not very fond of:

if (Answer == "Yes" || Answer == "yes" || Answer == "Yep" || Answer == "yep" || Answer == "Correct" || Answer == "correct" || Answer == "True" || Answer == "true")

not only because your line is too long but too many possibilities = ambiguity and confusion. This is not user-friendly. A console application should minimize the number of keystrokes required to interact with the program. So if the answer is yes or no, then it should be enough to answer Y or N. To read a single character from the console you can use Console.ReadKey().KeyChar. I suggest you have a look at this recent post of mine for suggestions about ergonomics.

Case-insensitive response

Don't do this:

if (Answer2 == "Name" || Answer2 == "name" || Answer2 == "My name" || Answer2 == "my name" || Answer2 == "My Name" || Answer2 == "The name" || Answer2 == "The Name" || Answer2 == "the name")

Convert the response to lowercase (String.ToLower), then check if the answer is 'name'.

String interpolation

Maybe I am influenced by F-strings in Python but instead of concatenating variables like this you can do string interpolation:

Console.Write("Ok, so your name is " + Name + " And you are " + Age + " Years old? ");

could be:

string str = $"Ok, so your name is {Name} And you are {Age} Years old?";
Console.WriteLine(str);

This is not a problem here. But there are situations where interpolation could improve readability, at least you are aware of this option.

Break up the code

I would break up the code in small functions. This code is too long and does many different things. It is a bit boring to read and there is lots of scrolling required to find an offending line. Breaking up the code in dedicated functions makes it easier to identify the portion of code responsible for something.

The other goal would be to simplify the structure of your program. The more you nest control blocks (if, while etc), the more it is susceptible to logic errors. And it's harder to follow the execution stream.

For example checking for minimum age could be a separate function that returns a boolean value. If it returns false, then you can branch out early.

Number validation should be made a function too. This is to avoid repetitive code and better separate functionality.

If you keep adding new functionality at some point the code will become unmanageable. Maybe you still have a good grasp on it but when you revisit it in 6 months ?

Console formatting

In your code you regularly switch the foreground color of the console:

Console.ForegroundColor = ConsoleColor.Red;
...
Console.ForegroundColor = ConsoleColor.Black;

I would write a small function to print messages, with a parameter indicating the severity (default = normal). Red for errors, Black for normal messages. And if you want to customize the colors, there will be only one function to change instead of doing search & replace over the whole code.

You'll make the code more flexible, less repetitive and reduce the number of lines of code. There is lots of repetition at present.

Misc

  • Most imports are unneeded. Normally the only one you need here is using System; and maybe Threading but I am not convinced it is useful
  • Numbers should be constrained within a certain range. Normally you will want positive numbers only and they should make sense. Age of 1000 is not a realistic answer and it should be declined.
  • You have lots of sleeps, I don't believe they bring any added value. They just slow down the user. This is because you sometimes clear the screen afterward but then the user may not have the time to see their mistake if they entered something wrong.
  • If the user enters an invalid value it would be good to be more explicit, for example say: expecting a number between 1 and 100. Printing "Please try again" is not very helpful. It is frustrating when a program or a web form rejects your input (for example a password) but doesn't tell you why and leaves you guessing.
\$\endgroup\$
1

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.