patterncsharpMinor
Retirement-saving calculator
Viewed 0 times
savingcalculatorretirement
Problem
I asked a math question and was hoping someone could have a look at the code for me as well. Unfortunately you might have to go look at the original question here.
Edit:
If I debug correctly, the PVg above (line repeated below) returns 0
Edit:
Here is the calculator: http://exceed.myib.co.za/calc
protected void ButtonCalRetire_Click(object sender, EventArgs e)
{
double FVc = 0;
double FVd = 0;
double PV = double.Parse(TextBoxCurrentSave.Text);
double i = double.Parse(TextBoxRoR.Text) / 100;
double b = (double)(int.Parse(TextBoxRetireAge.Text) - int.Parse(TextBoxCurrentAge.Text));
double R1 = double.Parse(TextBoxCurrentMonthlyContribution.Text);
double g2 = double.Parse(TextBoxAnnuityGrowthRetire.Text) / 100;
FVc = PV * Math.Pow((1 + i / 12), (b * 12));
FVd = R1 * (Math.Pow((1 + i / 12), (b * 12)) - Math.Pow((1 + g2 / 12), (b * 12))) / ((i / 12) - (g2 / 12));
double totSaved = FVc + FVd;
double R2 = double.Parse(TextBoxCurrentMonthlyContribution.Text);
double n = (double)(int.Parse(TextBoxDeathAge.Text) - int.Parse(TextBoxRetireAge.Text));
double PVg = R2 * Math.Pow((1 + g2 / 12), (b * 12)) / (i / 12 - g2 / 12) * (1 - ((Math.Pow((1 + g2 / 12), (n * 12))) / (Math.Pow((1 + i / 12), (n * 12)))));
double diff = FVc + FVd - PVg;
double R3 = diff / (Math.Pow((1 + i / 12), (b * 12)) - Math.Pow((1 + g2 / 12), (b * 12))) / ((i / 12) - (g2 / 12));
LabelTotControNeeded.Text = (R3 + R2).ToString("R ### ##0.00");
LabelControNeeded.Text = R3.ToString("R ### ##0.00");
}Edit:
If I debug correctly, the PVg above (line repeated below) returns 0
double PVg = R2 * Math.Pow((1 + g2 / 12), (b * 12)) / (i / 12 - g2 / 12) * (1 - ((Math.Pow((1 + g2 / 12), (n * 12))) / (Math.Pow((1 + i / 12), (n * 12)))));Edit:
Here is the calculator: http://exceed.myib.co.za/calc
Solution
Naming conventions
As you will have noticed from the coloring: you're not following naming conventions. Variables inside a method are lowerCamelCase which means this
becomes this
Naming
Your variables have very unclear names. From the context I know that this is part of a math formula, but you might be able to clarify them. If possible, consider using full words that clearly explain what that variable means.
In that regard I would definitely change this
into this
"Saving" 2 characters is nothing, but it lowers readability.
Parsing
You parse the same textbox fields many times. Instead, parse it once and put it a variable. Now you can reference that variable every time and it will be a lot easier to work with.
Formula clarity
You should also consider splitting up your long formulas (insofar a portion of a formula would make sense) to help you in case something needs to be debugged. It will be easier to check intermediate values and to keep the overview over what your formula represents.
As you will have noticed from the coloring: you're not following naming conventions. Variables inside a method are lowerCamelCase which means this
FVcbecomes this
fvCNaming
Your variables have very unclear names. From the context I know that this is part of a math formula, but you might be able to clarify them. If possible, consider using full words that clearly explain what that variable means.
In that regard I would definitely change this
totSavedinto this
totalSaved"Saving" 2 characters is nothing, but it lowers readability.
Parsing
You parse the same textbox fields many times. Instead, parse it once and put it a variable. Now you can reference that variable every time and it will be a lot easier to work with.
Formula clarity
You should also consider splitting up your long formulas (insofar a portion of a formula would make sense) to help you in case something needs to be debugged. It will be easier to check intermediate values and to keep the overview over what your formula represents.
Context
StackExchange Code Review Q#48091, answer score: 5
Revisions (0)
No revisions yet.