HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Calculator for more than two numbers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thanmorenumberstwoforcalculator

Problem

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.Write

Solution

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.

Context

StackExchange Code Review Q#43789, answer score: 9

Revisions (0)

No revisions yet.