Ytdyklly

I would like make this calculator program more Object Oriented but I'm struggling with how to do it. The calculator works perfectly but I would like to have another class that does the calculations and then sends that information back to the main class to be displayed. Any tips? I'm not sure how to call the methods and return the calculations.

 public partial class MainWindow : Window

    {

        //Basic Variables

        string input = string.Empty;

        string op1 = string.Empty;

        string op2 = string.Empty;

        char operation;

        double result = 0.0;



        public MainWindow()

        {

            InitializeComponent();

        }



        private void btnOn_Click(object sender, RoutedEventArgs e)

        {

            displayTextbox.IsEnabled = true;

        }



        private void btnOff_Click(object sender, RoutedEventArgs e)

        {

            displayTextbox.IsEnabled = false;

            displayTextbox.Text = String.Empty;

            displayTextbox.Text = "Off";

        }



        private void btn0_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 0;

            this.displayTextbox.Text += input;

        }



        private void btn1_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 1;

            this.displayTextbox.Text += input;

        }



        private void btn2_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 2;

            this.displayTextbox.Text += input;

        }



        private void btn3_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 3;

            this.displayTextbox.Text += input;

        }



        private void btn4_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 4;

            this.displayTextbox.Text += input;

        }



        private void btn5_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 5;

            this.displayTextbox.Text += input;

        }



        private void btn6_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 6;

            this.displayTextbox.Text += input;

        }



        private void btn7_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 7;

            this.displayTextbox.Text += input;

        }



        private void btn8_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 8;

            this.displayTextbox.Text += input;

        }



        private void btn9_Click(object sender, RoutedEventArgs e)

        {

            this.displayTextbox.Text = "";

            input += 9;

            this.displayTextbox.Text += input;

        }



        private void btnClear_Click(object sender, RoutedEventArgs e)

        {

            displayTextbox.Text = "";

            this.input = string.Empty;

            this.op1 = string.Empty;

            this.op2 = string.Empty;

        }



        private void btnAdd_Click(object sender, RoutedEventArgs e)

        {

            op1 = input;

            operation = '+';

            input = string.Empty;

        }



        private void btnDivision_Click(object sender, RoutedEventArgs e)

        {

            op1 = input;

            operation = '/';

            input = string.Empty;

        }



        private void btnMultiple_Click(object sender, RoutedEventArgs e)

        {

            op1 = input;

            operation = '*';

            input = string.Empty;

        }



        private void btnSubtract_Click(object sender, RoutedEventArgs e)

        {

            op1 = input;

            operation = '-';

            input = string.Empty;

        }



        private void btnEquals_Click(object sender, RoutedEventArgs e)

        {

            op2 = input;

            double num1, num2;

            double.TryParse(op1, out num1);

            double.TryParse(op2, out num2);



            if (operation == '+')

            {

                result = num1 + num2;

                displayTextbox.Text = result.ToString();

            }

            else if (operation == '-')

            {

                result = num1 - num2;

                displayTextbox.Text = result.ToString();

            }

            else if (operation == '*')

            {

                result = num1 * num2;

                displayTextbox.Text = result.ToString();

            }

            else if (operation == '/')

            {

                result = num1 / num2;

                displayTextbox.Text = result.ToString();

            }

        }

    }

Some tips from me:

1) You should separate the UI logic from your main logic. In your case, the logic is the calculation, so you should make a class for it. Don't think about the UI, just code your logic. It must be completely independent of the UI. This way you could test your calculation logic without creating windows every time and you could use another UI instead of the current one. You could even use the console.

2) Your class shouldn't inherit from Window. Instead, give an instance of Window to your class. One reason for this is, that it will pollute your MainWindows interface with many methods which you probably won't need. In general, favor Composition over inheritance.

3) You could define operation as a lambda:

// instead of:

operation = '-';

// use this:

operation = (a, b) => a - b;

It would make your btnEquals_Click method shorter and easier by removing the if-checks:

    private void btnEquals_Click(object sender, RoutedEventArgs e)

    {

        op2 = input;

        double num1, num2;

        double.TryParse(op1, out num1);

        double.TryParse(op2, out num2);

        displayTextbox.Text = (operation(num1, num2).toString(); // don't check, just apply

    }

4) You are repeating yourself multiple times:

this.displayTextbox.Text = "";

input += <some num>;

this.displayTextbox.Text += input;

Introduce a method that takes the number and does all this. It would reduce the amount of lines of code and generally: It's bad to have duplications because if it's wrong, you'll have to fix multiple places. See DRY.

5) I don't know what the initialize method does (used in the constructor), but I would advise you to stay away from this habit. A constructor should be lightweight and easy because as a user I don't want to get problems when I create your objects. I can understand that something might go wrong if I call a method because that's action and it can fail. But the creation of objects should be smooth and easy.

    private void btn0_Click(object sender, RoutedEventArgs e)

    {

        this.displayTextbox.Text = "";

        input += 0;

        this.displayTextbox.Text += input;

    }



    private void btn1_Click(object sender, RoutedEventArgs e)

    {

        this.displayTextbox.Text = "";

        input += 1;

        this.displayTextbox.Text += input;

    }



    //And 8 more...

It's very obvious that these methods are copy/paste and then you effectively change a single value.

A good rule of thumb is that a developer should count like a caveman: "one, two, many". When something is used more than twice, you need to abstract it into a reusable algorithm.

Doing so is, at first glance, very simple:

    private void numberButton_Click(object sender, RoutedEventArgs e)

    {

        this.displayTextbox.Text = "";

        input += 0;

        this.displayTextbox.Text += input;

    }

And then you set all your number buttons to have this same click event handler.

However, there is one issue: you need to know the input value of each button. This can be done in several ways. I'll list two: the quick and dirty way and the better way.

Quick and dirty

Since your button label equals the number you wish to add, you can effectively use that to get the right value. This isn't a great solution (it ties your UI to your logic), but it is the simplest solution I can think of:

    private void numberButton_Click(object sender, RoutedEventArgs e)

    {

        this.displayTextbox.Text = "";

        input += Convert.ToInt32((sender as Button)?.Text ?? "0");

        this.displayTextbox.Text += input;

    }

But this isn't the best solution. It feels (and demonstrably is) dirty.

Better

A better way would be to create a NumberButton class which works exactly like a button, but also contains a number value that you can use.

Create a new WPF user control. Change the XAML to use a <Button> tag instead of a <UserControl> tag, and set the code behind class to inherit from button:

public partial class NumberButton : Button {}

And then you add your additional value property:

public int NumberValue { get; set; }

In your calculator window XAML, you can now set this property:

<NumberButton ... NumberValue="1" Click="numberButton_Click" />

And then you can retrieve the NumberValue in the click event:

    private void numberButton_Click(object sender, RoutedEventArgs e)

    {

        this.displayTextbox.Text = "";

        input += (sender as NumberButton)?.NumberValue ?? 0;

        this.displayTextbox.Text += input;

    }

In case you were using WinForms instead of WPF, here's an MSDN guide on how to inherit from existing controls.