I'm learning to code for fun, and just finished my first program. I don't know what to think about it since it's my first finished real code, but hey it works! I'm pretty sure that it can be written shorter or better than this.

Right now it can only solve 1 problem at a time, but I'm going to make it so it can save the outcome and work further on and after that I'm of plan to change from using the console to having a real calculator look. I just want to improve myself by listening to your thoughts before working further on it.

What could I do shorter/easier/simpler/better?

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

namespace ConsoleApp1
{
    class Program
    {
    static void Main(string[] args)
    {
    ZoneNumber1: //First number
        Console.WriteLine();
        Console.WriteLine();
        Console.Write("Number: ");
        double Number1;
        double Number2;

        while (!double.TryParse(Console.ReadLine(), out Number1))
        {
            Console.Clear();
            Console.WriteLine();
            Console.WriteLine();
            Console.WriteLine("Wrong sort of input.");
            Console.Write("Enter a number: ");
        }
        Console.Clear();
        Console.WriteLine(Number1);
        Console.WriteLine();

        //Second number + action
    Action:
        Console.Write("Action: ");
        string Action = Console.ReadLine();
        Console.Clear();
        Console.WriteLine(Number1+""+Action);

        if ((Action == "*") || (Action == "/") || (Action == "-") || (Action == "+"))
        {
            switch (Action)
            {
                case "*": 
                    Console.WriteLine();
                    Console.Write("Number: ");
                    while (!double.TryParse(Console.ReadLine(), out Number2))
                    {
                        Console.Clear();
                        Console.WriteLine(Number1 + Action);
                        Console.WriteLine();
                        Console.WriteLine("Wrong sort of input.");
                        Console.Write("Enter a number: ");
                    }
                    Console.Clear();
                    Console.WriteLine(Number1 + "*" + Number2);
                    Console.WriteLine();
                    Console.WriteLine("= "+ Number1 * Number2);
                    Console.Write("Press enter to calculate again: "); Console.ReadLine();
                    Console.Clear();
                    goto ZoneNumber1;
                case "-":
                    Console.WriteLine();
                    Console.Write("Number: ");
                    while (!double.TryParse(Console.ReadLine(), out Number2))
                    {
                        Console.Clear();
                        Console.WriteLine(Number1 + Action);
                        Console.WriteLine();
                        Console.WriteLine("Wrong sort of input.");
                        Console.Write("Enter a number: ");
                    }
                    Console.Clear();
                    Console.WriteLine(Number1 + "-" + Number2);
                    Console.WriteLine();
                    Console.WriteLine("= " + (Number1 - Number2));
                    Console.Write("Press enter to calculate again: "); Console.ReadLine();
                    Console.Clear();
                    goto ZoneNumber1;
                case "+":
                    Console.WriteLine();
                    Console.Write("Number: ");
                    while (!double.TryParse(Console.ReadLine(), out Number2))
                    {
                        Console.Clear();
                        Console.WriteLine(Number1 + Action);
                        Console.WriteLine();
                        Console.WriteLine("Wrong sort of input.");
                        Console.Write("Enter a number: ");
                    }
                    Console.Clear();
                    Console.WriteLine(Number1 + "+" + Number2);
                    Console.WriteLine();
                    Console.WriteLine("= " + (Number1 + Number2));
                    Console.Write("Press enter to calculate again: "); Console.ReadLine();
                    Console.Clear();
                    goto ZoneNumber1;

                case "/":
                    Console.WriteLine();
                    Console.Write("Number: ");
                    while (!double.TryParse(Console.ReadLine(), out Number2))
                    {
                        Console.Clear();
                        Console.WriteLine(Number1 + Action);
                        Console.WriteLine();
                        Console.WriteLine("Wrong sort of input.");
                        Console.Write("Enter a number: ");
                    }
                    Console.Clear();
                    Console.WriteLine(Number1 + "/" + Number2);
                    Console.WriteLine();
                    Console.WriteLine("= " + (Number1 / Number2));
                    Console.Write("Press enter to calculate again: "); Console.ReadLine();
                    Console.Clear();
                    goto ZoneNumber1;
            }
        }

        else //if the input is not what it is supposed to be
        {
            Action = "";
            Console.Clear();
            Console.WriteLine(Number1 + Action);
            Console.WriteLine();
            Console.WriteLine("False action input, choose between: / ; * ; - ; +");
            goto Action;
        }

    }
  }
}
up vote 13 down vote accepted

Some rules of thumb:

  1. If you have your entire program in a single method (Main(), in this case), there are some design considerations you should probably revisit. Especially from a DRY ("Don't Repeat Yourself") perspective.

  2. If you find yourself using goto in a language invented after 1985, you're likely not availing yourself of the rich and varied control structures a language has to offer.

  3. If you're using C#, you should consider using object-oriented design and implementation. While the language does support other paradigms (such as the imperative form, which you're employing here), it's forte is OO and using it as a tool to solve problems is very much idiomatic.

  4. A good design approach, even in simple programs is to separate the so-called "business logic" from the user interface logic, also known as Separation of Concerns. Keep your calculations away from your WriteLines, as it were.

So, let's take a look at one possible refinement (I'm going to address points 1 and 2):

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

namespace ConsoleApp1
{
    class Program
    {
        static void Main(string[] args)
        {
            while (true)
            {
                double number1 = GetNumber();
                Console.Clear();
                Console.WriteLine(number1);
                Console.WriteLine();
                string action = GetAction();
                double number2 = GetNumber();
                Console.WriteLine(number2);
                double result = Perform(number1, action, number2);
                Console.Clear();
                Console.WriteLine(number1 + action + number2);
                Console.WriteLine();
                Console.WriteLine("= " + result);
                Console.Write("Press enter to calculate again: ");
                Console.ReadLine();
                Console.Clear();
            }
        }

        private static double GetNumber()
        {
            double number;

            Console.Write("Number: ");
            while (!double.TryParse(Console.ReadLine(), out number))
            {
                NumberInputError();
            }

            return number;
        }

        private static void NumberInputError()
        {
            Console.Clear();
            Console.WriteLine();
            Console.WriteLine();
            Console.WriteLine("Wrong sort of input.");
            Console.Write("Enter a number: ");
        }

        private static string GetAction()
        {
            while (true)
            {
                Console.Write("Action: ");
                string action = Console.ReadLine();
                Console.Clear();
                if ((action == "*") || (action == "/") || (action == "-") || (action == "+"))
                {
                    return action;
                }

                Console.WriteLine("False action input, choose between: / ; * ; - ; +");
            }
        }

        private static double Perform(double number1, string action, double number2)
        {
            switch (action)
            {
                case "+": return number1 + number2;
                case "-": return number1 - number2;
                case "*": return number1 * number2;
                case "/": return number1 / number2;
                default: throw new InvalidOperationException("False action input, choose between: / ; * ; - ; +");
            }
        }
    }
}
  • 1
    Thank you very much! You illuminated me by using private static s. Now I know what to learn next and how to improve. THANK YOU VERY MUCH! – Betrax Oct 19 at 20:06
  • 2
    Glad to help - hope to see more refinements and iterations along your journey. – Jesse C. Slicer Oct 19 at 20:07
  • 1
    Even languages invented before 1985 mostly have modern control structures in their current release. I'm curious to know how @betrax even knows gotos exist. – Isaac Rabinovitch Oct 20 at 6:06
  • @IsaacRabinovitch I was just scrolling through all possible codes that exist and found "goto", I thought that it was very flexible so decided to use. But from now on goto will be mine last choice of code. By looking at this code made by Jesse C. Slicer. I don't think that I'll need "goto" anymore. – Betrax Oct 20 at 10:49
  • 1
    @Betrax You might enjoy reading Dijkstra's original attack on gotos. cs.utexas.edu/users/EWD/transcriptions/EWD02xx/EWD215.html this led to the concept of structured programming (a term worth googling). This concept was controversial 40 years ago, but is now considered so fundamental that nobody even uses the term. – Isaac Rabinovitch Oct 21 at 20:27

I think your variant can be improved by reducing the number of code duplication.

My var:

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

namespace ConsoleApp1
{
    class Program
    {
        static double Number1;
        static string Action;
        static double Number2;
        static double Answer;
        static void Main(string[] args)
        {
            bool AppOn = true;

            while (AppOn != false)
            {
                Console.WriteLine();
                Console.WriteLine();
                Console.Write("Number: ");

                Number1 = TryGetNumber();

                Console.Clear();
                Console.WriteLine(Number1);


                Action = TryGetAction();

                Console.Clear();
                Console.WriteLine(Number1 + Action);


                Number2 = TryGetNumber();

                Console.Clear();
                Console.WriteLine(Number1 + Action + Number2);


                switch (Action)
                {
                    case "*":
                        Answer = Number1 * Number2;
                        break;
                    case "-":
                        Answer = Number1 - Number2;
                        break;
                    case "+":
                        Answer = Number1 + Number2;
                        break;
                    case "/":
                        Answer = Number1 / Number2;
                        break;
                }

                Console.WriteLine();
                Console.WriteLine("= " + Answer);
                Console.Write("Press enter to calculate again or esc for exit: ");
                ConsoleKeyInfo keyInfo = Console.ReadKey();
                if (keyInfo.Key == ConsoleKey.Enter)
                {
                    AppOn = true;
                }

                switch (keyInfo.Key)
                {
                    case ConsoleKey.Enter:
                        AppOn = true;
                        break;
                    case ConsoleKey.Escape:
                        AppOn = false;
                        break;
                }

                Console.Clear();
            }

        }

        public static double TryGetNumber()
        {
            double resultNumber;

            while (!double.TryParse(Console.ReadLine(), out resultNumber))
            {
                Console.Clear();
                Console.WriteLine();
                Console.WriteLine();
                Console.WriteLine("Wrong sort of input.");
                Console.Write("Enter a number: ");
            }

            return resultNumber;
        }

        public static string TryGetAction()
        {
            Console.WriteLine();
            Console.WriteLine();
            Console.Write("Action: ");

            string result = "";
            bool success = false;

            while (success != true)
            {
                string Action = Console.ReadLine();
                if ((Action == "*") || (Action == "/") || (Action == "-") || (Action == "+"))
                {
                    result = Action;
                    success = true;
                }
                else //if the input is not what it is supposed to be
                {
                    Action = "";
                    Console.Clear();
                    Console.WriteLine(Number1 + Action);
                    Console.WriteLine();
                    Console.WriteLine("False action input, choose between: / ; * ; - ; +");
                    success = false;
                }
            }
            return result;
        }
    }
}

What i do? Step by step.

  1. Remove all goto operator. In C# and in some (many) others lang decided that goto operator is evil operator, because in small programs you cant see this hell, but on projects larger code will become so hard for read and write. And in any case you use methods. There are many reasons, I advise you to read about this.
  2. Remove dublicate. In your version a lot of code that was created by Ctrl + C, Ctrl + V. Usually try to move away from this, wrapping such pieces in the methods and using several times. Let me show:(same color indicates same piece) enter image description here

Same pieces, but in my variant

enter image description here

  1. My version seems to be the same volume , because I added a lot of indents and spaces, focusing on the steps and structuring the code.

I use more things than in your version. I can answer your questions.

  • Thank you very much for this amazing detailed explanation. I'm just making sure that I understand everything you mentioned, (I am searching up some codes that I haven't learned yet" so it takes me some time to say something back. But I have to say that I wasn't hoping for something detailed as this so THANK YOU VERY MUCH! – Betrax Oct 19 at 20:29
  • 2
    Is there any particular reason to say "while (AppOn != false)" instead of "while(AppOn)"? – Isaac Rabinovitch Oct 20 at 6:15
  • @IsaacRabinovitch its my feature for select logic of my programm. In this string i say "while user dont off program".If i write "while(AppOn)" i say nothing. Yes, you can write it and its wirked, but my variatn(i think) better for bring to reader what i want. – Arantler Oct 22 at 10:55

You should not be repeating. Put it below the the final else.

while (!double.TryParse(Console.ReadLine(), out Number2))

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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