5
\$\begingroup\$

Recently I started learning programming and I created this program. I've added something to it with every new lesson.

How is my code-writing? Do I make mistakes I shouldn't make?

The program itself

static void Main()
    {
        {
            Console.WriteLine("Select language:\n");
            Console.WriteLine("1. Bulgarian");
            Console.WriteLine("2. English\n");
            Console.Write("Number: ");
            byte language = byte.Parse(Console.ReadLine());
        //....
        //Skipped to
        //English

        if (language == 2)
        {
            Console.Clear();
            string restart = "Press any key to exit the program...";
            Console.CursorVisible = true;
            string chislo = "Please press the number that corresponds to the function:";
            Console.WriteLine(chislo);
            Console.WriteLine();
            Console.WriteLine("1. Balance calculator");
            Console.WriteLine("2. Calculating numbers");
            Console.WriteLine("3. Comparing numbers");
            Console.WriteLine("4. Time and Date");
            Console.WriteLine("5. Calculation of the number of words and characters from a text/sentence \n");
            Console.WriteLine("Settings:\n");
            Console.WriteLine("6. Change the color of the console \n");
            Console.Write("Number: ");
            string chisloFunkciq = Console.ReadLine();
            int chisloFunkciqParse;
            if (int.TryParse(chisloFunkciq, out chisloFunkciqParse))
            {
                Console.Clear();

                //Funkciq Saldo

                if (chisloFunkciqParse == 1)
                {
                    Console.WriteLine("You chose \"Balance calculator\" \n");
                    bool check = true;
                    while (check == true)
                    {
                        Console.Write("Enter the opening balance: ");
                        string nachalnoSaldo = Console.ReadLine();
                        double chisloNachalnoSaldo;
                        if (double.TryParse(nachalnoSaldo, out chisloNachalnoSaldo))
                        {
                            Console.WriteLine();
                            Console.WriteLine("Your opening balance is {0:C} \n", chisloNachalnoSaldo);
                        }
                        else
                        {
                            Console.CursorVisible = false;
                            Console.Clear();
                            Console.Beep();
                            Console.WriteLine("\"{0}\" is not a number. Please try again. \n", nachalnoSaldo);
                            continue;
                        }

                        //Prihodi

                        Console.Write("Enter revenue: ");
                        string prihodi = Console.ReadLine();
                        Console.WriteLine();
                        double chisloPrihodi;
                        if (double.TryParse(prihodi, out chisloPrihodi))
                        {
                            Console.WriteLine("Your revenue is {0:C} \n", chisloPrihodi);
                        }
                        else
                        {
                            Console.CursorVisible = false;
                            Console.Clear();
                            Console.Beep();
                            Console.WriteLine("\"{0}\" is not a number. Please try again.\n", prihodi);
                            continue;
                        }

                        //Izrazhodvano saldo

                        Console.Write("Enter consumption balance: ");
                        string izrazhodvanoSaldo = Console.ReadLine();
                        Console.WriteLine();
                        double chisloKrainoSaldo;
                        if (double.TryParse(izrazhodvanoSaldo, out chisloKrainoSaldo))
                        {
                            Console.WriteLine("Your consumption balance is {0:C} \n", chisloKrainoSaldo);
                        }
                        else
                        {
                            Console.CursorVisible = false;
                            Console.Clear();
                            Console.Beep();
                            Console.WriteLine("\"{0}\" is not a number. Please try again. \n", izrazhodvanoSaldo);
                            continue;
                        }

                        //Rezultat

                        double rezultat = (chisloNachalnoSaldo + chisloPrihodi) - chisloKrainoSaldo;
                        Console.WriteLine("The remaining balance is {0:C} \n", rezultat);
                        check = false;
                        Console.CursorVisible = false;
                        Console.WriteLine(restart);
                        Console.ReadKey();
                        Console.Clear();
                    }
                }

                // Funkciq Calculator

                if (chisloFunkciqParse == 2)
                {
                    Console.WriteLine("You chose \"Calculating numbers\" \n");
                    double otgovor = 0;
                    Console.Write("Please enter the first number: ");
                    double purvoChislo = double.Parse(Console.ReadLine());
                    Console.WriteLine();
                    Console.Write("Please enter a character (+, -, * ili /): ");
                    string znak = Console.ReadLine();
                    Console.WriteLine();
                    Console.Write("Please enter the second number: \n");
                    double vtoroChislo = double.Parse(Console.ReadLine());

                    switch (znak)
                    {
                        case "+":
                            otgovor = purvoChislo + vtoroChislo;
                            break;
                        case "-":
                            otgovor = purvoChislo - vtoroChislo;
                            break;
                        case "*":
                            otgovor = purvoChislo * vtoroChislo;
                            break;
                        case "/":
                            otgovor = purvoChislo / vtoroChislo;
                            break;
                    }
                    Console.WriteLine("{0} {1} {2} = {3}", purvoChislo, znak, vtoroChislo, otgovor);
                    Console.WriteLine(restart);
                    Console.ReadKey();
                    Console.Clear();
                }

                //Sravnavane na chisla (2,3 ili 4)

                if (chisloFunkciqParse == 3)
                {
                    Console.Clear();
                    Console.WriteLine("You chose \"Comparing numbers\" \n");
                    Console.WriteLine("1. Comparing 2 numbers");
                    Console.WriteLine("2. Comparing 3 numbers");
                    Console.WriteLine("3. Comparing 4 numbers");
                    int chisloBroi = int.Parse(Console.ReadLine());

                    switch (chisloBroi)
                    {
                        case 1:
                            int sravPurvoChislo;
                            int sravVtoroChislo;
                            int sravTretoChislo;
                            int sravChetvurtoChislo;
                            Console.WriteLine("You chose \"Comparing 2 numbers\" \n");
                            Console.Write("Please enter the first number: ");
                            sravPurvoChislo = int.Parse(Console.ReadLine());
                            Console.WriteLine();
                            Console.WriteLine("Please enter the second number: ");
                            sravVtoroChislo = int.Parse(Console.ReadLine());
                            Console.WriteLine("{0} is bigger than {1}", Math.Max(sravPurvoChislo, sravVtoroChislo), Math.Min(sravPurvoChislo, sravVtoroChislo));
                            Console.WriteLine(restart);
                            Console.ReadKey();
                            Console.Clear();
                            break;
                        case 2:
                            Console.Clear();
                            Console.WriteLine("You chose \"Comparing 3 numbers\" \n");
                            Console.Write("Please enter the first number: ");
                            sravPurvoChislo = int.Parse(Console.ReadLine());
                            Console.WriteLine();
                            Console.Write("Please enter the second number: ");
                            sravVtoroChislo = int.Parse(Console.ReadLine());
                            Console.WriteLine();
                            Console.Write("Please enter the third number: ");
                            sravTretoChislo = int.Parse(Console.ReadLine());
                            Console.WriteLine("The largest of these numbers is {0}, and the smallest is {1}", Math.Max(Math.Max(sravPurvoChislo, sravVtoroChislo), sravTretoChislo), Math.Min(Math.Min(sravPurvoChislo, sravVtoroChislo), sravTretoChislo));
                            Console.WriteLine(restart);
                            Console.ReadKey();
                            Console.Clear();
                            break;
                        case 3:
                            Console.Clear();
                            Console.WriteLine("You chose \"Comparing 4 numbers\" \n");
                            Console.Write("Please enter the first number: ");
                            sravPurvoChislo = int.Parse(Console.ReadLine());
                            Console.WriteLine();
                            Console.Write("Please enter the second number: ");
                            sravVtoroChislo = int.Parse(Console.ReadLine());
                            Console.WriteLine();
                            Console.Write("Please enter the third number: ");
                            sravTretoChislo = int.Parse(Console.ReadLine());
                            Console.WriteLine();
                            Console.Write("Please enter the fourth number:  ");
                            sravChetvurtoChislo = int.Parse(Console.ReadLine());
                            int purviIzraz = Math.Max(sravPurvoChislo, sravVtoroChislo);
                            int vtoriIzraz = Math.Max(sravTretoChislo, sravChetvurtoChislo);
                            Console.WriteLine("The largest of these numbers is {0}, and the smallest is {1}", Math.Max(purviIzraz, vtoriIzraz), Math.Min(purviIzraz, vtoriIzraz));
                            Console.WriteLine(restart);
                            Console.ReadKey();
                            Console.Clear();
                            break;
                    }
                }

                //Funkciq Data

                if (chisloFunkciqParse == 4)
                {
                    Console.WriteLine("Today is {0} \n", DateTime.Now);
                    Console.WriteLine(restart);
                    Console.ReadKey();
                    Console.Clear();
                }

                //Funkciq Presmqtane na broi dumi i znaci ot tekst

                if (chisloFunkciqParse == 5)
                {
                    Console.Write("Please enter a text/sentence ");
                    string izrechenie = Console.ReadLine();
                    Console.WriteLine();
                    int broiDumi = izrechenie.Split(' ').Length;
                    int broiZnaci = izrechenie.Length;
                    if (broiDumi == 1)
                    {
                        Console.Write("This text contains {0} word and ", broiDumi);
                    }
                    else
                    {
                        Console.Write("The text contains {0} words and ", broiDumi);
                    }
                    if (broiZnaci == 1)
                    {
                        Console.WriteLine("{0} character", broiZnaci);
                    }
                    else
                    {
                        Console.WriteLine("{0} characters", broiZnaci);
                    }
                    Console.WriteLine();
                    Console.WriteLine(restart);
                    Console.ReadKey();
                    Console.Clear();
                }

                //Cvqt Konzola

                if (chisloFunkciqParse == 6)
                {
                    bool check = true;
                    while (check == true)
                    {
                        Console.WriteLine("Choose the number corresponding to the color you want:\n");
                        Console.WriteLine("1. Red");
                        Console.WriteLine("2. Green");
                        Console.WriteLine("3. Yello");
                        Console.WriteLine("4. Return to the previous settings (Black)\n");
                        Console.Write("Number: ");
                        string cvqtChislo = Console.ReadLine();
                        int cvqtChisloParse;
                        if (int.TryParse(cvqtChislo, out cvqtChisloParse))
                        {
                            switch (cvqtChisloParse)
                            {
                                case 1:
                                    Console.BackgroundColor = ConsoleColor.Red;
                                    Console.ForegroundColor = ConsoleColor.Black;
                                    Console.Clear();
                                    Console.WriteLine("You chose red");
                                    Console.WriteLine();
                                    Main();
                                    break;
                                case 2:
                                    Console.BackgroundColor = ConsoleColor.Green;
                                    Console.ForegroundColor = ConsoleColor.Black;
                                    Console.Clear();
                                    Console.WriteLine("You chose green");
                                    Console.WriteLine();
                                    Main();
                                    break;
                                case 3:
                                    Console.BackgroundColor = ConsoleColor.Yellow;
                                    Console.ForegroundColor = ConsoleColor.Black;
                                    Console.Clear();
                                    Console.WriteLine("You chose yellow");
                                    Console.WriteLine();
                                    Main();
                                    break;
                                case 4:
                                    Console.BackgroundColor = ConsoleColor.Black;
                                    Console.ForegroundColor = ConsoleColor.Gray;
                                    Console.Clear();
                                    Console.WriteLine("You chose the default color (black)");
                                    Console.WriteLine();
                                    Main();
                                    break;
                                default:
                                    Console.CursorVisible = false;
                                    Console.Clear();
                                    Console.Beep();
                                    break;
                            }
                            continue;
                        }
                        else
                        {
                            Console.CursorVisible = false;
                            Console.Clear();
                            Console.Beep();
                        }
                    }
                }

                if (chisloFunkciqParse > 6)
                {
                    Console.CursorVisible = false;
                    Console.Beep();
                    Console.Clear();

                    Main();
                }
            }
            else
            {
                Console.Clear();
                Console.WriteLine("You did not specify a number. Please try again \n");
                Console.CursorVisible = false;
                Console.Beep();
                Main();
            }

        }
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Hey there. How about putting up specific parts of your code you would like reviewed. I think you will find this question will get closed otherwise. If there are lots of parts you would like reviewed you could make multiple questions etc \$\endgroup\$
    – dreza
    Feb 14, 2014 at 19:40
  • \$\begingroup\$ Indeed right, @dreza. For more information as to why, see this meta-question We would love to review your code, new user, but please extract some parts of the code and post them within the question itself :) \$\endgroup\$ Feb 14, 2014 at 19:45
  • 1
    \$\begingroup\$ That's better, thank you, new user! Just by looking over the code quickly I think there's several things that can be simplified in it. One of our C# reviewers will probably assist you shortly :) Thank you for flying Code Review airlines, we hope you have a pleasant flight! \$\endgroup\$ Feb 14, 2014 at 19:54

2 Answers 2

7
\$\begingroup\$

One thing that I don't like about your code is the recursive call to Main(); inside of Main().

I don't think this is healthy, if the application is run for an extended period of time, or run over and over again you will have issues with memory management.

What happens is the first time through you start a thread and then never fully release the thread before you call another thread, so you lock the memory that was being used and it isn't deallocated/destroyed.

Instead of doing this, you should wrap the entire application in a single while loop based on a Boolean variable that you can change to false when you want to exit the program.


I am not completely sure what is going on here.

static void Main()
    {
        {
            Console.WriteLine("Select language:\n");
            Console.WriteLine("1. Bulgarian");
            Console.WriteLine("2. English\n");
            Console.Write("Number: ");
            byte language = byte.Parse(Console.ReadLine());
        //....
  1. Why is there two starting Brackets?
  2. Why do you indent the first bracket?
    • is this preference in the coding?
    • personally this would confuse me a little bit

I would write it like this

static void Main()
{
    Console.WriteLine("Select language:\n");
    Console.WriteLine("1. Bulgarian");
    Console.WriteLine("2. English\n");
    Console.Write("Number: ");
    byte language = byte.Parse(Console.ReadLine());
        //....

And with that, I think that you are missing a bracket at the end if you intentionally doubled up the bracket (I think doubling up the brackets might irritate the compiler; I have never tried so I don't know for sure).

Other than that, the syntax looks good and isn't missing any semi-colons, and doesn't have any other weird indentations.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Thanks for the suggestions! As for the brackets, I deleted some of the code so it can fit here and I didn't delete the brackets, sorry about that :) I've never made such a big program and things have turned into a spaghetti bowl, but I am getting used to it. So buy adding a loop you mean creating this: bool check = true while (check == true) { //include everything here ? } \$\endgroup\$
    – Milenchy
    Feb 14, 2014 at 20:59
  • \$\begingroup\$ Yes. Inside the block you can alter check so that the code can exit the loop when you need it to. \$\endgroup\$
    – Malachi
    Feb 14, 2014 at 21:07
  • 1
    \$\begingroup\$ Ok I think I get it, thank you very much :) \$\endgroup\$
    – Milenchy
    Feb 14, 2014 at 21:15
5
\$\begingroup\$

I would at least break out the functions listed in the second menu into their own methods. With option 1 : Balance Calculator for example, you could break out all of the code within this statement:

if (chisloFunkciqParse == 1)  
{
    RunBalanceCalculator();
}

private void RunBalanceCalculator()
{
    //TODO Paste everything that was originally in the above if statement.
}

This will at least break your code out into logical groupings by function. If this were a real system and not just a learning program, I would also recommend that you break out presentation from the logic as well, but that is for after you get more familiar with object oriented programming.

Also, some of the code could be condensed so that you're following the DRY principle. For example, the code for option 3 could be made dynamic with a function like this:

private void CompareNumbers(params int[] numbers)
{
    //TODO Write the algorithm to compare all of the numbers in the array
}    

Then, you can call this method from each of the case statements condensing similar code into one place making it easier to maintain if there is a bug. This will also bring forth the need for validating the input you get. The current code looks like it would break if the user typed a non-numeric character.

\$\endgroup\$
2
  • \$\begingroup\$ All of my +1. The first thing I thought on seeing this method was 'Dear lord, that thing is huge!' \$\endgroup\$
    – Magus
    Feb 14, 2014 at 23:38
  • \$\begingroup\$ Haha! Ok, I'll do that. \$\endgroup\$
    – Milenchy
    Feb 15, 2014 at 8:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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