C++ - Problem with functions

[Necrix]

I kinda put off asking for help with this because it's kinda embarrassing.
The code works as it is until I try to put it into functions.
So, with everything in the main() function, it works.
This is a project for class and while I understand the concept..I'm not sure exactly what to do.
When I'm done, I shouldn't have any variables declared globally.

Any advice?
Thanks.

By the way, I tried to comment as much as I could incase there was alot of "wth, why'd you do that"s lol.

Code: Select all

//NecrixWolf
//2010

//The purpose of this program is to demonstrate compounding interest in a 
//"bank account"

//This program is meant to receive input from the user:
       //How much money to begin with
       //Which month to begin with
       //What rate of interest will be applied to each month
       //How many months should be calculated
//The following should be the output:
      //The current month
      //The amount of interest earned this month
      //The new(total) balance
      //This will be looped up to the number of months

//Three functions are used to achieve this: input, calculate, and output
      
#include <iostream.h>
#include <string.h>
#include <iomanip.h> //To use for isdigit() error handling later
#include <ctype.h>
using namespace std;

 //Function prototypes
 void get_input();
 void calculate();
 void output(int count);
 
 //Global variables
 float rate;
 double money, inter, total = 0;
 bool error = false; //To be used later for error handling..maybe.
 
 //To convert counter value into month output for the user
 string one = "January",
 two = "February",
 three = "March",
 four = "April",
 five = "May",
 six = "June",
 seven = "July",
 eight = "August",
 nine = "September",
 ten = "October",
 eleven = "November",
 twelve = "December"; 
 
//Begin main function 
int main()
{
 get_input(); 
 for (int count = start; count <= (months +1); count++)
 {
 calculate();
 output(count);
 }

 system("PAUSE");
 return 0;   
}

//Input function gets user's input
void get_input()
{ 
 cout << "How much money to start?: $";
 cin >> money;     
 cout << "How many months are you going to be calculating?: ";
 cin >> months;
 cout << "What month are you starting at?: ";
 cin >> start;  //Uses "start" variable instead of "count" for code readability
 cout << "What is the interest rate for each month?: %";
 cin >> rate; 
 cout << '\n';
 rate = rate / 100; //Divides given rate by 100 so the user can enter 
                    //rate of interest in percent form
 }


//Calculates..duh
 void calculate()
 {
 inter = money * rate; //Calculates amount of interest earned for current month
 money = money * (1 + rate); //Calculates the amount of money for the month before
                             //interest is added
 total += money; //Finds the total for each month
 }
 
 //Guess what this one does..
 void output(int count)
 {
 cout << "For the month of: ";
 //Uses a switch to "convert" the counter's value into the month for output
 switch(count)
 {
 case 1: cout << one;
 break;
 case 2: cout << two;
 break;
 case 3: cout << three;
 break;
 case 4: cout << four;
 break;
 case 5: cout << five;
 break;
 case 6: cout << six;
 break;
 case 7: cout << seven;
 break;
 case 8: cout << eight;
 break;
 case 9: cout << nine;
 break;
 case 10: cout << ten;
 break;
 case 11: cout << eleven;
 break;
 case 12: cout << twelve;
 break;
 }
 cout.setf(ios::fixed);
 cout << "\nYour interest is: $" << setprecision(2) << inter;
 cout << "\nYour new balance is: $" << setprecision(2) << total << "\n\n";
 }    

[leetnigga]

Your program is written in a very imperative way. There are dependencies between functions everywhere and most of them modify global variables, which will influence the working of other functions.

You can make multi-line comments by enclosing the text in /* and */.

Here is how I would do it:

Code: Select all

#include <iostream>
#include <cstring>
#include <iomanip>
#include <cctype>
#include <cassert>
using namespace std;

double get_initial_balance()
{
  double initial_balance;
  cout << "Initial balance: ";
  cin >> initial_balance;
  return initial_balance;
}

int get_starting_month()
{
  int starting_month;
  cout << "Starting month: ";
  cin >> starting_month;
  return starting_month;
}

int get_ending_month()
{
  int ending_month;
  cout << "Ending month: ";
  cin >> ending_month;
  return ending_month;
}

float get_interest_rate()
{
  float interest_rate;
  cout << "Interest rate: ";
  cin >> interest_rate;
  interest_rate /= 100;
  return interest_rate;
}

string month_string(int month_index)
{
  assert(month_index >= 1 && month_index <= 12);
  string months[] = {"January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"};
  return months[month_index-1];
}

int main()
{
  double balance, interest;
  int month_start, month_end;
  float interest_rate;

  balance = get_initial_balance();
  month_start = get_starting_month();
  month_end = get_ending_month();
  interest_rate = get_interest_rate();

  assert(month_start >= 1 && month_end <= 12);

  cout << endl;

  cout.setf(ios::fixed); 
  int month;
  for(month = month_start;month <= month_end;month++) {
    cout << "For the month of " << month_string(month) << endl;
    interest = balance * interest_rate;
    cout << "Your interest is: " << setprecision(2) << interest << endl;
    balance *= (1+interest_rate);
    cout << "Your new balance is: " << setprecision(2) << balance << endl << endl;
  }

  return 0;
}

My separation into functions may seem like overkill especially for simply returning some input, but this makes it much easier to add validation for example, without affecting the rest of the program.

I don't think getting the calculation out of main() would do much good. Most of it is for output, and all the calculations depend on each other.

(The results need to be displayed sequentially anyway. Using the intermediate result for the next calculation is fine.)

If you did separate the calculation it would be a void function, or it would return some result of the calculation like the final balance. It wouldn't really add anything, because most of the value is in the output anyway.

I'd say my implementation is pretty optimal.

[Necrix]

O_O It's so...clean!!
Thanks alot, man!

I'll have to wait to get a closer look at it since I have to go to work in a few minutes, though.

[leetnigga]

You're welcome!

I hope it gives you a few ideas of how to separate things to make your code cleaner.

I wrote the code in increments. First I wrote the part that read in the initial balance. That made me realize it should be a separate function, so I factored it out of main() and placed a call to the new function there instead. Each time making sure none of the functions influence each other, trying not to depend on global state.

A few improvements could be made. get_starting_month() and get_ending_month() could be replaced by something like get_month_interval(). get_initial_balance() can be generalized to get_balance() and get_interest_rate() to get_percentage(). I see an economics library emerging!

A good way to write programs I think is to create a sort of domain-specific-language for your application. For a compound interest calculator program that interacts with a user, you want types that represent percentages (float) and money (double). You want functions to read a balance from the user, or a month or a percentage. Now that I think of it, the word "read" would be more appropriate than "get", and you may want to make those functions operate on a stream or character buffer for more flexibility. That will help you test (and write unit tests for) your program more easily.

All of those things may not be a big deal in this small case, but they become much more important when your program grows.