WPF Calculator Code

Question

I've coded a calculator in C# with WPF as the UI.

I wish to know mainly about these points:

Ways of optimizing
Better techniques, tactics and ways of coding this
All flaws on the surface as well as deep
Simpler logic

MainWindow.xaml.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;

namespace NewCalculator
{
    public partial class MainWindow : Window
    {
        static int[] numbersArray = new int[10];
        static string[] operatorsArray = new string[9];

        static string storageVariable;
        static int numbersCounter = 0;
        static int operatorsCounter = 0;
        static int total = 0;
        static bool totalled = false;

        public MainWindow()
        {
            InitializeComponent();
        }

        private void One_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "1";
            storageVariable += "1";
        }
        private void Two_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "2";
            storageVariable += "2";
        }
        private void Three_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "3";
            storageVariable += "3";
        }
        private void Four_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "4";
            storageVariable += "4";
        }
        private void Five_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "5";
            storageVariable += "5";
        }
        private void Six_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "6";
            storageVariable += "6";
        }
        private void Seven_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "7";
            storageVariable += "7";
        }
        private void Eight_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "8";
            storageVariable += "8";
        }
        private void Nine_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "9";
            storageVariable += "9";
        }
        private void Zero_Click(object sender, RoutedEventArgs e)
        {
            if (totalled == true)
            {
                Display.Content = "";
                totalled = false;
            }
            Display.Content += "0";
            storageVariable += "0";
        }
        private void Add_Click(object sender, RoutedEventArgs e)
        {
            setNumber(storageVariable);
            setOperator("+");
            Display.Content += "+";
        }
        private void Subtract_Click(object sender, RoutedEventArgs e)
        {
            setNumber(storageVariable);
            setOperator("-");
            Display.Content += "-";
        }
        private void Multiply_Click(object sender, RoutedEventArgs e)
        {
            setNumber(storageVariable);
            setOperator("*");
            Display.Content += "x";
        }
        private void Divide_Click(object sender, RoutedEventArgs e)
        {
            setNumber(storageVariable);
            setOperator("/");
            Display.Content += "/";
        }
        private void Equal_Click(object sender, RoutedEventArgs e)
        {
            setNumber(storageVariable);
            for (int i = 0; i < operatorsCounter; i++)
            {
                if (operatorsArray[i] == "+" && i == 0)
                {
                    total = numbersArray[i] + numbersArray[i + 1];
                }
                else if (operatorsArray[i] == "+")
                {
                    total = total + numbersArray[i + 1];
                }
                else if (operatorsArray[i] == "-" && i == 0)
                {
                    total = numbersArray[i] - numbersArray[i + 1];
                }
                else if (operatorsArray[i] == "-")
                {
                    total = total - numbersArray[i + 1];
                }
                else if (operatorsArray[i] == "*" && i == 0)
                {
                    total = numbersArray[i] * numbersArray[i + 1];
                }
                else if (operatorsArray[i] == "*")
                {
                    total = total * numbersArray[i + 1];
                }
                else if (operatorsArray[i] == "/" && i == 0)
                {
                    total = numbersArray[i] / numbersArray[i + 1];
                }
                else if (operatorsArray[i] == "/")
                {
                    total = total / numbersArray[i + 1];
                }
            }
            Display.Content = total;
            numbersArray = null;
            operatorsArray = null;
            storageVariable = null;
            numbersCounter = 0;
            operatorsCounter = 0;
            total = 0;
            totalled = true;
        }
        static void setNumber(String Number)
        {
            numbersArray[numbersCounter] = Convert.ToInt16(Number);
            storageVariable = null;
            numbersCounter++;
        }
        static void setOperator(String Op)
        {
            operatorsArray[operatorsCounter] = Op;
            operatorsCounter++;
        }
        private void AC_Click(object sender, RoutedEventArgs e)
        {
            Display.Content = "";
            numbersArray = null;
            operatorsArray = null;
            storageVariable = null;
            numbersCounter = 0;
            operatorsCounter = 0;
            total = 0;
        }
    }
}

MainWindow.xaml

    <Window x:Class="NewCalculator.MainWindow"
            xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
            xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
            Title="Calculator" Height="200" Width="195" ResizeMode="NoResize">
        <Grid>
            <Label x:Name="Display" Content="" HorizontalAlignment="Left" Height="32" Margin="10,3,0,0" VerticalAlignment="Top" Width="138" BorderThickness="1" RenderTransformOrigin="0.543,1.375" />
            <Button x:Name="One" Content="1" HorizontalAlignment="Left" Height="20" Margin="10,40,0,0" VerticalAlignment="Top" Width="20" Click="One_Click"/>
            <Button x:Name="Two" Content="2" HorizontalAlignment="Left" Height="20" Margin="40,40,0,0" VerticalAlignment="Top" Width="20" Click="Two_Click"/>
            <Button x:Name="Three" Content="3" HorizontalAlignment="Left" Height="20" Margin="70,40,0,0" VerticalAlignment="Top" Width="20" Click="Three_Click"/>
            <Button x:Name="Four" Content="4" HorizontalAlignment="Left" Height="20" Margin="10,70,0,0" VerticalAlignment="Top" Width="20" Click="Four_Click"/>
            <Button x:Name="Five" Content="5" HorizontalAlignment="Left" Height="20" Margin="40,70,0,0" VerticalAlignment="Top" Width="20" Click="Five_Click"/>
            <Button x:Name="Six" Content="6" HorizontalAlignment="Left" Height="20" Margin="70,70,0,0" VerticalAlignment="Top" Width="20" Click="Six_Click"/>
            <Button x:Name="Seven" Content="7" HorizontalAlignment="Left" Height="20" Margin="10,100,0,0" VerticalAlignment="Top" Width="20" Click="Seven_Click"/>
            <Button x:Name="Eight" Content="8" HorizontalAlignment="Left" Height="20" Margin="40,100,0,0" VerticalAlignment="Top" Width="20" Click="Eight_Click"/>
            <Button x:Name="Nine" Content="9" HorizontalAlignment="Left" Height="20" Margin="70,100,0,0" VerticalAlignment="Top" Width="20" Click="Nine_Click"/>
            <Button x:Name="Zero" Content="0" HorizontalAlignment="Left" Height="20" Margin="40,130,0,0" VerticalAlignment="Top" Width="20" Click="Zero_Click"/>
            <Button x:Name="Add" Content="+" HorizontalAlignment="Left" Height="20" Margin="100,40,0,0" VerticalAlignment="Top" Width="20" Click="Add_Click"/>
            <Button x:Name="Subtract" Content="-" HorizontalAlignment="Left" Height="20" Margin="130,40,0,0" VerticalAlignment="Top" Width="20" Click="Subtract_Click"/>
            <Button x:Name="Multiply" Content="x" HorizontalAlignment="Left" Height="20" Margin="100,70,0,0" VerticalAlignment="Top" Width="20" Click="Multiply_Click"/>
            <Button x:Name="Divide" Content="/" HorizontalAlignment="Left" Height="20" Margin="130,70,0,0" VerticalAlignment="Top" Width="20" Click="Divide_Click"/>
            <Button x:Name="Equal" Content="=" HorizontalAlignment="Left" Height="20" Margin="100,100,0,0" VerticalAlignment="Top" Width="20" Click="Equal_Click"/>
            <Button x:Name="AC" Content="AC" Height="20" Margin="130,100,17,0" VerticalAlignment="Top" Width="30" Click="AC_Click"/>
        </Grid>
    </Window>

If you changed from using events to using commands, you could include a specific parameter, which means they could all share the same method. That alone might clean things up a lot. — Magus, Jul 23 '14 at 14:46
In this question, all the answers are correct. The one I ticked is because he is talking about 2 topics while others 1. — Hassan Althaf, Jul 24 '14 at 6:48

Pimgd · Accepted Answer · 2014-07-23 15:03:39Z

Duplication

if (totalled == true)
{
    Display.Content = "";
    totalled = false;
}
Display.Content += "1";
storageVariable += "1";

if (totalled == true)
{
    Display.Content = "";
    totalled = false;
}
Display.Content += "2";
storageVariable += "2";

All the way up to 0. (1-9 + 0) If you can somehow figure out which button caused the click event, you can make 1 click function for all your numeric buttons. You can also do this for your +, -, / and * buttons.

Bugs

private void AC_Click(object sender, RoutedEventArgs e)
{
    Display.Content = "";
    numbersArray = null;//null pointer exception on pressing any of the arithmetic buttons
    operatorsArray = null;//null pointer exception on pressing any of the arithmetic buttons
    storageVariable = null;//null pointer exception on pressing any of the numeric buttons
    numbersCounter = 0;
    operatorsCounter = 0;
    total = 0;
}

Don't set to null, that leads to things breaking. AC clears the current calculation on a calculator - so set the values back to the initial values.

static void setNumber(String Number)
{
    numbersArray[numbersCounter] = Convert.ToInt16(Number);//no bounds check
    storageVariable = null;//possible null pointer when clicking arithmetic button
    numbersCounter++;
}
static void setOperator(String Op)
{
    operatorsArray[operatorsCounter] = Op;//no bounds check
    operatorsCounter++;
}

You're missing bounds checks. Because you've limited the amount of numbers and operations that are allowed, it's possible to get an out of range exception (or whatever C#'s exception is for going out of an array's bounds).

Magus · Answer 2 · 2014-07-23 21:06:41Z

Commands

In WPF, one very valuable property is the Command property.

For instance:

<Button
  x:Name="One"
  Content="1"
  HorizontalAlignment="Left"
  Height="20"
  Margin="10,40,0,0"
  VerticalAlignment="Top"
  Width="20"
  Click="One_Click"
/>

can be changed to:

<Button
  x:Name="One"
  Content="1"
  HorizontalAlignment="Left"
  Height="20"
  Margin="10,40,0,0"
  VerticalAlignment="Top"
  Width="20"
  Command="ClickCommand"
  CommandParameter="1"
/>

Usually, I'd do this with a binding to a viewmodel, but if the command is a member of the code behind, it may still work. Two and onward should only involve changing the parameter.

private ICommand clickCommand; // This will get you a lazily assigned command.
public ICommand ClickCommand { get { return clickCommand ?? (clickCommand = new <WhateverYouNameYourCommandClass>(Click); } }
// In this example, the command takes a delegate of type Action<string>

private void Click(string parameter)
{
  int integerValue;

  if(int.TryParse(parameter, out integerValue))
  {
    if (totalled == true)
    {
      Display.Content = "";
      totalled = false;
    }
    Display.Content += parameter;
    storageVariable += parameter;
  }
}

From there, you can either write another command for your function buttons or use else ifs. This at least replaces nine whole methods with a single, simpler one.

Note that if the above xaml does not work, what you need to do is change the Command property to this: Command="{Binding RelativeSource={RelativeSource AncestorType={x:Type yourXamlNamespace:MainWindow}}, Path=ClickCommand} or set up your data context properly.

This is what I meant - if you can figure out how to pass a parameter per button, you can make all the buttons use the same function! — Pimgd, Jul 24 '14 at 7:34
@Pimgd: Yeah, I figured that if the other two answers were specific but left this open, I'd better fill it. Commands are very useful. I'm slightly spoiled because I'm used to the Prism DelegateCommand which supports a second delegate which enables or disables the button. — Magus, Jul 24 '14 at 14:38

Xiaoy312 · Answer 3 · 2014-07-23 19:25:14Z

Access Modifier

Normally you want to avoid using static unless you want the field, property or method to be available without instancing. Generally, a static field or property is used for application-wide settings. While static method is usually a stateless helper method, such as : Math.Cos, Int.Parse.

So I would remove the static on these lines, as these are tied the instance of the calculator. It wouldn't make sense to have 2 instances of the calculator that share the same memory, so that any input on one will be synchronized on the other.

    static int[] numbersArray = new int[10];
    static string[] operatorsArray = new string[9];

    static string storageVariable;
    static int numbersCounter = 0;
    static int operatorsCounter = 0;
    static int total = 0;
    static bool totalled = false;

and

    static void setNumber(String Number)
    {
        numbersArray[numbersCounter] = Convert.ToInt16(Number);
        storageVariable = null;
        numbersCounter++;
    }
    static void setOperator(String Op)
    {
        operatorsArray[operatorsCounter] = Op;
        operatorsCounter++;
    }

asked	9 months ago
viewed	1376 times
active	2 months ago

current community

your communities

more stack exchange communities

WPF Calculator Code

3 Answers 3

Commands

Access Modifier

Your Answer

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

Hot Network Questions

current community

your communities

more stack exchange communities

WPF Calculator Code

3 Answers 3

Commands

Access Modifier

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions