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

Calculator with WPF

Submitted by: @import:stackexchange-codereview··
0
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

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

  • 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 from App.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.