patterncsharpMinor
Refactoring Calculations through a series of textboxes
Viewed 0 times
refactoringcalculationsthroughseriestextboxes
Problem
Alright guys, I know this isn't the proper way of problem solving, But I need some guidance here. Since I am just starting in programming, the code is not really readable.
The Winforms program (essentially a glorified Excel sheet) I created is made up from different textboxes (as shown in designer picture below). The textboxes are used for input of numbers. All the input is extracted, converted to int or double and run through the calculation. The textboxes on the right hand side (orange boxes) are used to show the calculated price to the users.
The questions are:
How do I make the code more readable by encapsulating and inheritance?
What do I use to make a list of every variable and how do I make a standard calculation and loop every pair of textboxes through the same calculation?
```
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Drawing.Printing;
using System.Drawing.Imaging;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
partial class BudgyMobilePlan : Form
{
//declaring the variables used in the calculation
int ContractDuration1;
int ContractDuration2;
int ContractDuration3;
int ContractDuration4;
int ContractDuration5;
int ContractDuration6;
double MonthlyFee1;
double MonthlyFee2;
double MonthlyFee3;
double MonthlyFee4;
double MonthlyFee5;
double MonthlyFee6;
double PhonePrice1;
double PhonePrice2;
double PhonePrice3;
double PhonePrice4;
double PhonePrice5;
double PhonePrice6;
double AdditionalCost1;
double AdditionalCost2;
double AdditionalCost3;
double AdditionalCost4;
double AdditionalCost5;
double AdditionalCost6;
private void CalculatedCostButton_Click(object sender, EventArgs e)
{
boo
The Winforms program (essentially a glorified Excel sheet) I created is made up from different textboxes (as shown in designer picture below). The textboxes are used for input of numbers. All the input is extracted, converted to int or double and run through the calculation. The textboxes on the right hand side (orange boxes) are used to show the calculated price to the users.
The questions are:
How do I make the code more readable by encapsulating and inheritance?
What do I use to make a list of every variable and how do I make a standard calculation and loop every pair of textboxes through the same calculation?
```
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Drawing.Printing;
using System.Drawing.Imaging;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
partial class BudgyMobilePlan : Form
{
//declaring the variables used in the calculation
int ContractDuration1;
int ContractDuration2;
int ContractDuration3;
int ContractDuration4;
int ContractDuration5;
int ContractDuration6;
double MonthlyFee1;
double MonthlyFee2;
double MonthlyFee3;
double MonthlyFee4;
double MonthlyFee5;
double MonthlyFee6;
double PhonePrice1;
double PhonePrice2;
double PhonePrice3;
double PhonePrice4;
double PhonePrice5;
double PhonePrice6;
double AdditionalCost1;
double AdditionalCost2;
double AdditionalCost3;
double AdditionalCost4;
double AdditionalCost5;
double AdditionalCost6;
private void CalculatedCostButton_Click(object sender, EventArgs e)
{
boo
Solution
Honestly, it looks like you want to use a domain object to model your providers and perform the pay calculations. Rather than using a bunch of text fields and drop-down controls, you can then data bind your domain object to a
Domain Object
You want an object which has properties for all your display values. Since data binding is involved, you need to ensure the object implements the
For the plan type, you can simply use an enumeration:
Without knowing your domain, I just threw in placeholders for illustration purposes.
Data Binding
If your list of providers is fixed, you can bind any
I would then add a BindingSource to your form which points to the
The
Pay Calculation
The calculation step is now a simple matter of enumerating your providers and calling
That's assuming, of course, that you even want to wait to calculate pay.
Another alternative which might be more usable would be to calculate on-the-fly as users enter values. To do this, you add a call to
For this to work out, you have to tweak the
As a result, you could eliminate the Calculate button.
The end result is that your code-behind for the form looks something more like the following (minus the click handler if you decided to go with auto-updati
DataGridView.Domain Object
You want an object which has properties for all your display values. Since data binding is involved, you need to ensure the object implements the
INotifyPropertyChanged interface, like so:public sealed class Provider : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
public string Name
{
get { return _name; }
set
{
_name = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Name)));
}
}
public PlanType PlanType
{
get { return _planType; }
set
{
_planType = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(PlanType)));
}
}
public double Duration
{
get { return _duration; }
set
{
_duration = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Duration)));
}
}
public double MonthlyFee
{
get { return _monthlyFee; }
set
{
_monthlyFee = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(MonthlyFee)));
}
}
public double Price
{
get { return _price; }
set
{
_price = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Price)));
}
}
public double AdditionalCost
{
get { return _additionalCost; }
set
{
_additionalCost = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(AdditionalCost)));
}
}
public double Pay { get { return _pay; } }
public void CalculatePay()
{
var months = Duration * 12;
_pay = months * MonthlyFee + Price + AdditionalCost / months;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Pay)));
}
private string _name;
private PlanType _planType;
private double _duration;
private double _monthlyFee;
private double _price;
private double _additionalCost;
private double _pay;
}For the plan type, you can simply use an enumeration:
public enum PlanType
{
CoolPlan,
BadPlan,
GoodPlan,
}Without knowing your domain, I just threw in placeholders for illustration purposes.
Data Binding
If your list of providers is fixed, you can bind any
IEnumerable to the grid. However, in the more likely case you want to allow adding new providers, I suggest instantiating and binding to a BindingList.I would then add a BindingSource to your form which points to the
Provider type, then set the DataSource property of the grid to the binding source, and tweak your columns to look how you like. In your code-behind, you will set the DataSource property on this BindingSource to your collection of providers.The
PlanType field required a little extra care. In this case, you want to set up the column as a DataGridViewComboBoxColumn. Once you do that, you want to supply the possible list of values for PlanType to the column so the drop-down works as expected. This can be done easily with Enum.GetValues:var availableTypes = Enum.GetValues(typeof(PlanType));
planTypeDataGridViewTextBoxColumn.DataSource = availableTypes;Pay Calculation
The calculation step is now a simple matter of enumerating your providers and calling
CalculatePay:private void calculateButton_Click(object sender, EventArgs e)
{
foreach(var provider in _providers)
{
provider.CalculatePay();
}
}That's assuming, of course, that you even want to wait to calculate pay.
Another alternative which might be more usable would be to calculate on-the-fly as users enter values. To do this, you add a call to
CalculatePay in Provider's property setters:public double Duration
{
get { return _duration; }
set
{
_duration = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Duration)));
CalculatePay();
}
}For this to work out, you have to tweak the
CalculatePay method to handle zero values:public void CalculatePay()
{
if (Duration == 0)
{
_pay = 0;
}
else
{
var months = Duration * 12;
_pay = months * MonthlyFee + Price + AdditionalCost / months;
}
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Pay)));
}As a result, you could eliminate the Calculate button.
The end result is that your code-behind for the form looks something more like the following (minus the click handler if you decided to go with auto-updati
Code Snippets
public sealed class Provider : INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;
public string Name
{
get { return _name; }
set
{
_name = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Name)));
}
}
public PlanType PlanType
{
get { return _planType; }
set
{
_planType = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(PlanType)));
}
}
public double Duration
{
get { return _duration; }
set
{
_duration = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Duration)));
}
}
public double MonthlyFee
{
get { return _monthlyFee; }
set
{
_monthlyFee = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(MonthlyFee)));
}
}
public double Price
{
get { return _price; }
set
{
_price = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Price)));
}
}
public double AdditionalCost
{
get { return _additionalCost; }
set
{
_additionalCost = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(AdditionalCost)));
}
}
public double Pay { get { return _pay; } }
public void CalculatePay()
{
var months = Duration * 12;
_pay = months * MonthlyFee + Price + AdditionalCost / months;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Pay)));
}
private string _name;
private PlanType _planType;
private double _duration;
private double _monthlyFee;
private double _price;
private double _additionalCost;
private double _pay;
}public enum PlanType
{
CoolPlan,
BadPlan,
GoodPlan,
}var availableTypes = Enum.GetValues(typeof(PlanType));
planTypeDataGridViewTextBoxColumn.DataSource = availableTypes;private void calculateButton_Click(object sender, EventArgs e)
{
foreach(var provider in _providers)
{
provider.CalculatePay();
}
}public double Duration
{
get { return _duration; }
set
{
_duration = value;
PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Duration)));
CalculatePay();
}
}Context
StackExchange Code Review Q#134995, answer score: 8
Revisions (0)
No revisions yet.