Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am having this problem with my code. I have variables firstnumber and secondnumber, as well as operand and result. Then I am using the string.Split to know whether it's an operator or a number.

After that I have used a while loop if a.Length is over 2 and if it is, I declared an double numbers which is a[i]. i is 3 and gets + 2 every time the while loop executes.

Then I declared a variable called operands which is a[b]. b = 4 and gets + 2 every time the loop executes. Then I want to compare the string operands and the string operator, to see if it is +, -, * or / in the end I have put an if statement, so that if a[i] == null, then the while loop breaks and the result is printed.

I know my code is messy, and I have probably done many things wrong here, so I would really appreciate every word of advice I can get.

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

namespace ConsoleApplication1
{
 class Calculator
 {
     static void Main(string[] args)
     {
        Console.WriteLine("This is a calculator. enter the numbers this way: Example(4 + 3)");
        Console.WriteLine("!Use space between numbers!");
        start : string s = Console.ReadLine(); 
        overagain:  string [] avgränsare = { " " };
        string[] a = s.Split(avgränsare, StringSplitOptions.RemoveEmptyEntries); 

        while (a.Length > 2)
        {
            int i = 3;
            int b = 4;
            double numbers = double.Parse(a[i]);
            i = i + 2;
            b = b + 2;

            double firstnumber = double.Parse(a[0]);
            string operand = a[1];
            double secondnumber = double.Parse(a[2]);
            double result = 0;
            string operands = a[b];
            goto overagain;

                if (a[i] == null)
                {
                  Console.WriteLine(result);
                  break;
                }
            }

        Console.WriteLine("Do you want to continue?");
        s = Console.ReadLine().ToLower();
        if (s == "y")
        {
            goto start;
        }
        else
        {
        }

        }
    }
 }
share|improve this question
    
Welcome to Code Review! I have written an answer that should help you clean up your code. Unfortunately, you shouldn't expect any answers to extend the functionality of your code. Code Review is for cleaning up existing code. Code Review is not for adding features to, or modify the results of, existing code. –  Simon André Forsberg Mar 8 at 12:53
1  
xkcd.com/292 –  palacsint Mar 8 at 15:26

1 Answer 1

Using goto is not a good practice!!

Instead of using goto, use a do-while or a while loop. You want to do something as long as the answer to "Do you want to continue" is "Yes". This is perfect for a do-while loop.

You should also avoid using goto overagain and instead use a for-loop to iterate through the array. It could also be solved with recursion. Use anything you want except the goto statement!


Make your code self-documenting. Your "explanation" of your code in the beginning of your question confuses me more than helps me.

Self-document your code by using proper variable names. s can be called input instead, and i and b can be called something that explains what they are doing. Right now, I can't exactly understand what they are supposed to do.


Your last block of Console.WriteLine("Do you want to continue?");, including the if-else that comes afterwards should be indented one step further.


If you don't have anything to do in else, just skip that part. if ... { ... } is enough, you don't need else. You'd better remove this, or else...


Use English names for variables, especially when characters outside the range of A-Z, a-z, 0-9, _ are included (such as "ä"). The Swedish word "avgränsare" translates to "delimiter" in English.

share|improve this answer
    
Thanks. However i posted this in stackoverflow, and they Said i should go to this place. –  Rickard B. Mar 8 at 13:44
1  
@RickardB. That is most likely because your question is very unclear, and you are asking for a "cleanup" (which is on-topic here). Please show me the link of your StackOverflow question and I can see if there are any differences. Either way, if you would ask "how can I add this functionality to my code" on StackOverflow then they would likely tell you to try something first and then come back and ask a specific question. –  Simon André Forsberg Mar 8 at 13:53

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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