patterncsharpMinor
Calculator with WPF
Viewed 0 times
withwpfcalculator
Problem
I built a calculator in C# with WPF for the UI. I'm still a beginner at WPF and C# in general, and I'm just looking for some constructive criticism.
XAML
C#
`using System;
using System.Collections.Generic;
using System.Linq;
using System.Windows;
using System.Windows.Controls;
namespace Calculator
{
public partial class MainWindow: Window
{
Calculator c;
public MainWindow()
{
InitializeComponent();
this.Loaded += OnLoad;
}
void OnLoad(object sender, RoutedEventArgs e)
{
c = new Calculator(currentEquation);
AddClickEvents();
}
void AddClickEvents()
{
b1.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(1);
};
b2.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(2);
};
b3.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(3);
};
b4.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(4);
};
b5.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(5);
};
b6.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(6);
};
b7.Cl
XAML
C#
`using System;
using System.Collections.Generic;
using System.Linq;
using System.Windows;
using System.Windows.Controls;
namespace Calculator
{
public partial class MainWindow: Window
{
Calculator c;
public MainWindow()
{
InitializeComponent();
this.Loaded += OnLoad;
}
void OnLoad(object sender, RoutedEventArgs e)
{
c = new Calculator(currentEquation);
AddClickEvents();
}
void AddClickEvents()
{
b1.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(1);
};
b2.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(2);
};
b3.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(3);
};
b4.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(4);
};
b5.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(5);
};
b6.Click += (object sender, RoutedEventArgs e) =>
{
c.Press(6);
};
b7.Cl
Solution
The main problem with your code is that it is hard to test.
The view is tightly coupled to the logic of calculating.
I would like to start with recommending the MVVM pattern for any WPF application. It has a good separation of the View and the ViewModel and makes your code more testable.
I would like to recommend the Prism library for helping with the MVVM pattern.
I would try to avoid the static keyword as it makes your code harder to test.
Check out this demo project for MVVM in prism:
https://code.msdn.microsoft.com/windowsdesktop/MVVM-Code-Sample-using-the-7e59b7bb
Edit
I also found a bug in your solution. If you do a operation on the same number for example 5 + 5 your application throws an error. That is because you are using a dictionary and it can't contain the same key twice.
If you would have written your app in MVVM you could unit test your ViewModel.
I would also use decimal instead of float. It is better with precision.
https://stackoverflow.com/questions/6341855/how-to-decide-what-to-use-double-or-decimal
Edit 2 - MVVM demo to help you get started
Demo Code - not complete:
The view is tightly coupled to the logic of calculating.
I would like to start with recommending the MVVM pattern for any WPF application. It has a good separation of the View and the ViewModel and makes your code more testable.
I would like to recommend the Prism library for helping with the MVVM pattern.
I would try to avoid the static keyword as it makes your code harder to test.
Check out this demo project for MVVM in prism:
https://code.msdn.microsoft.com/windowsdesktop/MVVM-Code-Sample-using-the-7e59b7bb
Edit
I also found a bug in your solution. If you do a operation on the same number for example 5 + 5 your application throws an error. That is because you are using a dictionary and it can't contain the same key twice.
If you would have written your app in MVVM you could unit test your ViewModel.
I would also use decimal instead of float. It is better with precision.
https://stackoverflow.com/questions/6341855/how-to-decide-what-to-use-double-or-decimal
Edit 2 - MVVM demo to help you get started
- Create a new Wpf application
- Add the NuGet package Prism.MVVM
- Use dependency injection for testability and program against interfaces.
- Delete this
StartupUri="MainWindow.xaml"from fromApp.xaml
- Add code below to help you get started
Demo Code - not complete:
public partial class App : Application
{
protected override void OnStartup(StartupEventArgs e)
{
var window = new CalculatorWindow(new CalculatorViewModel(new Calculator()));
window.Show();
}
}
public class CalculatorViewModel : BindableBase
{
private readonly Calculator calculator;
private string currentValue;
public CalculatorViewModel(Calculator calculator)
{
this.calculator = calculator;
}
public string CurrentValue
{
get { return currentValue; }
set
{
currentValue = value;
OnPropertyChanged(() => CurrentValue);
}
}
public ICommand OneCommand
{
get { return new DelegateCommand(() => AddNumberToInput(1)); }
}
public ICommand TwoCommand
{
get { return new DelegateCommand(() => AddNumberToInput(2)); }
}
public ICommand ThreeCommand
{
get { return new DelegateCommand(() => AddNumberToInput(3)); }
}
public ICommand FourCommand
{
get { return new DelegateCommand(() => AddNumberToInput(4)); }
}
public ICommand FiveCommand
{
get { return new DelegateCommand(() => AddNumberToInput(5)); }
}
public ICommand SixCommand
{
get { return new DelegateCommand(() => AddNumberToInput(6)); }
}
public ICommand SevenCommand
{
get { return new DelegateCommand(() => AddNumberToInput(7)); }
}
public ICommand EightCommand
{
get { return new DelegateCommand(() => AddNumberToInput(8)); }
}
public ICommand NineCommand
{
get { return new DelegateCommand(() => AddNumberToInput(9)); }
}
public ICommand ZeroCommand
{
get { return new DelegateCommand(() => AddNumberToInput(0)); }
}
public ICommand DotCommand
{
get { return new DelegateCommand(AddDotToInput); }
}
public ICommand PlusCommand
{
get
{
return new DelegateCommand(() =>
{
calculator.AddNumber(double.Parse(CurrentValue));
calculator.AddOperation(Operation.Plus);
});
}
}
public ICommand MinusCommand { get; private set; }
public ICommand MultiplyCommand { get; private set; }
public ICommand DevideCommand { get; private set; }
public ICommand EqualsCommand { get; private set; }
public ICommand DeleteCommand { get; private set; }
public ICommand ClearCommand { get; private set; }
private void AddNumberToInput(int number)
{
CurrentValue += number;
}
private void AddDotToInput()
{
CurrentValue += ".";
}
}
Code Snippets
public partial class App : Application
{
protected override void OnStartup(StartupEventArgs e)
{
var window = new CalculatorWindow(new CalculatorViewModel(new Calculator()));
window.Show();
}
}
public class CalculatorViewModel : BindableBase
{
private readonly Calculator calculator;
private string currentValue;
public CalculatorViewModel(Calculator calculator)
{
this.calculator = calculator;
}
public string CurrentValue
{
get { return currentValue; }
set
{
currentValue = value;
OnPropertyChanged(() => CurrentValue);
}
}
public ICommand OneCommand
{
get { return new DelegateCommand(() => AddNumberToInput(1)); }
}
public ICommand TwoCommand
{
get { return new DelegateCommand(() => AddNumberToInput(2)); }
}
public ICommand ThreeCommand
{
get { return new DelegateCommand(() => AddNumberToInput(3)); }
}
public ICommand FourCommand
{
get { return new DelegateCommand(() => AddNumberToInput(4)); }
}
public ICommand FiveCommand
{
get { return new DelegateCommand(() => AddNumberToInput(5)); }
}
public ICommand SixCommand
{
get { return new DelegateCommand(() => AddNumberToInput(6)); }
}
public ICommand SevenCommand
{
get { return new DelegateCommand(() => AddNumberToInput(7)); }
}
public ICommand EightCommand
{
get { return new DelegateCommand(() => AddNumberToInput(8)); }
}
public ICommand NineCommand
{
get { return new DelegateCommand(() => AddNumberToInput(9)); }
}
public ICommand ZeroCommand
{
get { return new DelegateCommand(() => AddNumberToInput(0)); }
}
public ICommand DotCommand
{
get { return new DelegateCommand(AddDotToInput); }
}
public ICommand PlusCommand
{
get
{
return new DelegateCommand(() =>
{
calculator.AddNumber(double.Parse(CurrentValue));
calculator.AddOperation(Operation.Plus);
});
}
}
public ICommand MinusCommand { get; private set; }
public ICommand MultiplyCommand { get; private set; }
public ICommand DevideCommand { get; private set; }
public ICommand EqualsCommand { get; private set; }
public ICommand DeleteCommand { get; private set; }
public ICommand ClearCommand { get; private set; }
private void AddNumberToInput(int number)
{
CurrentValue += number;
}
private void AddDotToInput()
{
CurrentValue += ".";
}
}
<Window x:Class="MVVMCalculator.CalculatorWindow"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
Title="Calculator" Height="350" Width="350">
<Window.Resources>
<Style TargetType="Button">
Context
StackExchange Code Review Q#101003, answer score: 7
Revisions (0)
No revisions yet.