patterncsharpMinor
WPF Calculator Code
Viewed 0 times
codewpfcalculator
Problem
I've coded a calculator in C# with WPF as the UI.
I wish to know mainly about these points:
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(
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(
Solution
Duplication
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
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.
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).
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).
Code Snippets
if (totalled == true)
{
Display.Content = "";
totalled = false;
}
Display.Content += "1";
storageVariable += "1";
if (totalled == true)
{
Display.Content = "";
totalled = false;
}
Display.Content += "2";
storageVariable += "2";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;
}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++;
}Context
StackExchange Code Review Q#57806, answer score: 8
Revisions (0)
No revisions yet.