Messy C++ style. Any suggestions?

  • (5 Pages)
  • +
  • 1
  • 2
  • 3
  • 4
  • 5

61 Replies - 2638 Views - Last Post: 09 September 2012 - 11:40 AM Rate Topic: -----

#31 ZenithMkII  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 49
  • Joined: 06-September 12

Re: Messy C++ style. Any suggestions?

Posted 07 September 2012 - 05:46 PM

Wow, that was easy. So, the problem was that I was using a
option = getChoice();
to update the option at the end of the while loop, but since the while loop has a
(option = getChoice()) != Quit
I didn't need to update anymore.

This is what it looks like so far.

account.h
#ifndef ACCOUNT_H
#define ACCOUNT_H

class Account{
private:
	double balance;
	char accountType;
	double errorOverdraft(double);
	double errorNegative(double);
public:
	Account() : balance(0){};
	Account(double bal) : balance(bal){};
	void setBalance(double);
	double getBalance() const;
	void setAccountType(char);
	char getAccountType() const;
	void subtract(double);
	void add(double);
	void transfer(Account, double);
	void displayBalance() const;
	void withdraw(double);
	void deposit(double);
};

#endif


account.cpp
#include "account.h"
#include <iostream>
#include <iomanip>
#include <string>

using namespace std;

//Checks if the amount is negative and above the balance.
double Account::errorOverdraft(double amount){
	while(amount > balance){
		cout << "\nInvalid amount. Amount exceeds balance.\nEnter another amount: ";
		cin  >> amount;
	}
	return amount;
}

//Checks if the amount is negatvie.
double Account::errorNegative(double amount){
	while(amount < 0){
		cout << "\nInvalid amount. Amount is a negative number.\nEnter another amount: ";
		cin  >> amount;
	}
	return amount;
}

void Account::setBalance(double bal){
	balance = bal;
}

double Account::getBalance() const{
	return balance;
}

void Account::subtract(double amount){
	amount = errorOverdraft(errorNegative(amount));
	balance -= amount;
}

//Added error checking inside of this function so call do not add a negative number.
void Account::add(double amount){
	amount = errorNegative(amount);
	balance += amount;
}

//Added error checking to see if amount is negative or too high.
void Account::transfer(Account acc, double amount){
	amount = errorOverdraft(errorNegative(amount));
	balance -= amount;
	acc.add(amount);
}

void Account::displayBalance() const{
	cout << "$" << fixed << setprecision(2) << getBalance();
}

//Takes in an Account class object and subtracts the amount from its balance.
void Account::withdraw(double amount){
	amount = errorNegative(errorOverdraft(amount));
	balance -= amount;
}

//Takes in an Account class object and adds the amount to its balance.
void Account::deposit(double amount){
	amount = errorNegative(amount);
	balance += amount;
}


main.cpp
#include <iostream>
#include <iomanip>
#include <string>
#include "account.h"

using namespace std;

double getAmount();
enum AccountType{Savings, Checkings};
enum MenuOption{Withdraw, Deposit, Transfer, Balance, Quit};
MenuOption getChoice();
AccountType selectAccount();
void showMenu();
void showAccountMenu();

int main(){
	//Initialization
	Account savings(5000), checkings(5000);
	MenuOption option;
	AccountType accountType;
	string choice;
	double amount;

	//Show balance update.
	cout << "\nSavings: ";
	savings.displayBalance();
	cout << "  Checkings: ";
	checkings.displayBalance();

	//Initial read

	while((option = getChoice()) != Quit){
		accountType = selectAccount();

		Account * thisAccount;
		Account * thatAccount;

		if(accountType == Savings){
			thisAccount = &savings;
			thatAccount = &checkings;
		}
		else{
			thisAccount = &checkings;
			thatAccount = &savings;
		}

		switch(option){
		case Withdraw:
			amount = getAmount();
			thisAccount->subtract(amount);
			break;
		case Deposit:
			amount = getAmount();
			thisAccount->add(amount);
			break;
		case Transfer:
			amount = getAmount();
			thisAccount->withdraw(amount);
			thatAccount->deposit(amount);
			break;
		case Balance:
			thisAccount->displayBalance();
			break;
		}

		//Show balance update.
		cout << "\nSavings: ";
		savings.displayBalance();
		cout << "  Checkings: ";
		checkings.displayBalance();
	}
	return 0;
}

//This function takes in a string and returns 'W', 'D', 'T', 'B', or 'Q' to decide the choice.
MenuOption getChoice(){
	string choice;

	showMenu();
	cin >> choice;
		
	while(choice[0] != 'w' && choice[0] != 'W' && choice[0] != 'd' && choice[0] != 'D' && choice[0] != 't' && 
		  choice[0] != 'T' && choice[0] != 'b' && choice[0] != 'B' && choice[0] != 'q' && choice[0] != 'Q'){
		cout << "\nInvalid selection.\n";
		showMenu();
		cin >> choice;
	}

	switch (choice[0]){
		case 'w':
		case 'W':
			cout << "\nWithdraw selected.\n";
			return Withdraw;
			break;
		case 'd':
		case 'D':
			cout << "\nDeposit selected.\n";
			return Deposit;
			break;
		case 't':
		case 'T':
			cout << "\nTransfer selected.\n";
			return Transfer;
			break;
		case 'b':
		case 'B':
			cout << "\nShow balance selected.\n";
			return Balance;
			break;
		case 'q':
		case 'Q':
			cout << "\nQuitting.\n";
			return Quit;
			break;
	}
}

//This function returns 'S' or 'C' for savings or checkings.
AccountType selectAccount(){
	string acc;

	showAccountMenu();
	cin >> acc;
	while(acc[0] != 's' && acc[0] != 'S' && acc[0] != 'c' && acc[0] != 'C'){
		cout << "\nInvalid account selection.\n";
		showAccountMenu();
		cin >> acc;
	}

	switch (acc[0]){
		case 'c':
		case 'C':
			cout << "\nCheckings selected.\n";
			return Checkings;
			break;
		case 's':
		case 'S':
			cout << "\nSavings selected.\n";
			return Savings;
			break;
	}
}

//Assigns a value to amount.
double getAmount(){
	double amount;
	cout << "\nEnter an amount: ";
	cin >> amount;
	return amount;
}

void showMenu(){
	cout << "\n\n[W]ithdraw\n"
		 << "[D]eposit\n"
		 << "[T]ransfer\n"
		 << "Show [B]alance\n"
		 << "[Q]uit\n";
}

void showAccountMenu(){
	cout << "\nAccount Select\n\n"
		 << "[S]avings\n"
		 << "[C]heckings\n";
}





Was This Post Helpful? 1
  • +
  • -

#32 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3162
  • View blog
  • Posts: 9,541
  • Joined: 05-May 12

Re: Messy C++ style. Any suggestions?

Posted 07 September 2012 - 07:03 PM

You seem to have an unused getAccountType() and setAccountType() on your account class.

Additionally, you can use toupper() to convert the user input so that you don't have to keep comparing against both the upper and lower case versions of the character.

And as suggested before, you can find string<>::find_first_of() to simplify your while condition on line 82.
Was This Post Helpful? 1
  • +
  • -

#33 ZenithMkII  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 49
  • Joined: 06-September 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 02:10 AM

View PostSkydiver, on 07 September 2012 - 07:03 PM, said:

You seem to have an unused getAccountType() and setAccountType() on your account class.

Additionally, you can use toupper() to convert the user input so that you don't have to keep comparing against both the upper and lower case versions of the character.

And as suggested before, you can find string<>::find_first_of() to simplify your while condition on line 82.

About the unused member functions, I was going to implement jimblumberg's accountType as a private data member.
I saw your earlier post, but I didn't know how to implement the find.
With the string<>::, is this the only way to implement it? With the :: (scope) operator it looks like the std::. Is it possible to use the using namespace with it?
Also, I really like the toupper() idea.

I will try to get this included. Thank you.

This post has been edited by ZenithMkII: 08 September 2012 - 02:11 AM

Was This Post Helpful? 0
  • +
  • -

#34 ZenithMkII  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 49
  • Joined: 06-September 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 02:32 AM

Just found this little section of code from

http://www.cplusplus.../find_first_of/

// string::find_first_of
#include <iostream>
#include <string>
using namespace std;

int main ()
{
  string str ("Replace the vowels in this sentence by asterisks.");
  size_t found;

  found=str.find_first_of("aeiou");
  while (found!=string::npos)
  {
    str[found]='*';
    found=str.find_first_of("aeiou",found+1);
  }

  cout << str << endl;

  return 0;
}


There are some things I don't get.

1. Where does the size_t data type come from? Why is it used?
2. What is the meaning behind the while loop's condition?
3. What is string::npos?
4. What is happening in the update part of the while loop?

I've also added the toupper() and it has made things a lot shorter. With this find_first_of() my while loops will be looking more neat. Also, keep in mind that when entering a number, the user actually puts in a number and not a letter. There probably is a way to catch this also. I would like to explore this.

Thanks

This post has been edited by ZenithMkII: 08 September 2012 - 02:40 AM

Was This Post Helpful? 0
  • +
  • -

#35 ZenithMkII  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 49
  • Joined: 06-September 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 03:45 AM

Here is my while loop.

string choice, selection("WDTBQ");

showMenu();
cin >> choice;

while(toupper(choice[0]) == selection[selection.find_first_of("WDTBQ")]){
	cout << "\nInvalid selection.\n";
	showMenu();
	cin >> choice;
}


For some reason I can't enter anything to enter the withdraw choice at all, but it continues correctly for all the other choices.

Maybe I'm using the find_front_of() wrong.

This post has been edited by ZenithMkII: 08 September 2012 - 03:45 AM

Was This Post Helpful? 0
  • +
  • -

#36 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3162
  • View blog
  • Posts: 9,541
  • Joined: 05-May 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 06:55 AM

I think you are locked into one perspective of using the find_first_of() function after seeing that sample code from: http://www.cplusplus.../find_first_of/

Try this point of view:
bool IsVowel(char ch)
{
    string vowels("AEIOU");

    return vowels.find_first_of(toupper(ch)) != string::npos;
}


Was This Post Helpful? 1
  • +
  • -

#37 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3162
  • View blog
  • Posts: 9,541
  • Joined: 05-May 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 07:04 AM

View PostZenithMkII, on 08 September 2012 - 02:32 AM, said:

1. Where does the size_t data type come from? Why is it used?

It is a C/C++ data type that can hold the maximum number of bytes. See more details at: http://www.cplusplus...cstddef/size_t/

View PostZenithMkII, on 08 September 2012 - 02:32 AM, said:

2. What is the meaning behind the while loop's condition?

See #3.

View PostZenithMkII, on 08 September 2012 - 02:32 AM, said:

3. What is string::npos?

It's a constant to indicate that something wasn't found. From that same documentation, see:

Quote

Return Value
The position of the first occurrence in the string of any of the characters searched for.
If the content is not found, the member value npos is returned.

More info at: http://www.cplusplus...ng/string/npos/

View PostZenithMkII, on 08 September 2012 - 02:32 AM, said:

4. What is happening in the update part of the while loop?

Line 14 is a simple assignment to replace the vowel found with an asterisk.
Was This Post Helpful? 1
  • +
  • -

#38 jimblumberg  Icon User is offline

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,735
  • Joined: 25-December 09

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 09:38 AM

Quote

1. Where does the size_t data type come from? Why is it used?

The size_t is an implementation defined unsigned type that is large enough to hold the size of any object. You use this type when you want to insure you can safely access the largest object size. For example a std::string can hold size_t::max() characters. You should also use this type to hold values returned from functions that return a size_t.

Quote

2. What is the meaning behind the while loop's condition?

while (found!=string::npos)

This loop will execute while found is not equal to std::string::npos.

Quote

3. What is string::npos?

The std::string::npos returns the largest value that can be held in a size_t. This value is the value returned in the case of a failure in the string find operations.

Jim
Was This Post Helpful? 1
  • +
  • -

#39 ZenithMkII  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 49
  • Joined: 06-September 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 01:04 PM

View PostSkydiver, on 08 September 2012 - 06:55 AM, said:

I think you are locked into one perspective of using the find_first_of() function after seeing that sample code from: http://www.cplusplus.../find_first_of/

Try this point of view:
bool IsVowel(char ch)
{
    string vowels("AEIOU");

    return vowels.find_first_of(toupper(ch)) != string::npos;
}


So, find_first_of() returns a size_t datatype. Then, if find_first_of() != string::npos it returns the size_t number.

Since the IsVowel() is a bool return type any number that is not 0 is true?

If this is correct, then when find_first_of() == string::npos it equals 0? Or does it just return a 0 when it == string::npos?

I'm just trying to figure out how this works. I got it in my code and it works, but I do not fully understand what's going on.

This post has been edited by ZenithMkII: 08 September 2012 - 01:07 PM

Was This Post Helpful? 0
  • +
  • -

#40 jimblumberg  Icon User is offline

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,735
  • Joined: 25-December 09

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 01:15 PM

Quote

So, find_first_of() returns a size_t datatype. Then, if it != string::npos it returns the size_t number.

Yes find_first_of() returns a size_t datatype, but more importantly it returns std::string::npos if it fails to find the characters in the string. You are comparing this value returned to std::string::npos. You returning the result of this comparison, which is a bool. It may be easier to visualize if you add the parentheses to the return statement:
return(vowels.find_first_of(toupper(ch)) != string::npos);

Or maybe split this statement into it's substeps:

ch = toupper(ch);
size_t position = vowels.find_first_of(ch);
bool found = position != std::string::npos;
return(found);


Jim

This post has been edited by jimblumberg: 08 September 2012 - 01:16 PM

Was This Post Helpful? 1
  • +
  • -

#41 ZenithMkII  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 49
  • Joined: 06-September 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 01:17 PM

Here's the code I've edited. Looks really nice. Thanks to all of your help!

#include <iostream>
#include <iomanip>
#include <string>
#include "account.h"

using namespace std;

double getAmount();
enum AccountType{Savings, Checkings};
enum MenuOption{Withdraw, Deposit, Transfer, Balance, Quit};
MenuOption getChoice();
AccountType selectAccount();
void showMenu();
void showAccountMenu();
void showBalance(Account, Account);
bool isMenuChoice(string);
bool isAccountChoice(string);

int main(){
	//Initialization
	Account savings(5000), checkings(5000);
	MenuOption option;
	AccountType accountType;
	string choice;
	double amount;

	showBalance(savings, checkings);

	//Initial read

	while((option = getChoice()) != Quit){
		accountType = selectAccount();

		Account * thisAccount;
		Account * thatAccount;

		if(accountType == Savings){
			thisAccount = &savings;
			thatAccount = &checkings;
		}
		else{
			thisAccount = &checkings;
			thatAccount = &savings;
		}

		switch(option){
		case Withdraw:
			amount = getAmount();
			thisAccount->subtract(amount);
			break;
		case Deposit:
			amount = getAmount();
			thisAccount->add(amount);
			break;
		case Transfer:
			amount = getAmount();
			thisAccount->withdraw(amount);
			thatAccount->deposit(amount);
			break;
		case Balance:
			thisAccount->displayBalance();
			break;
		}

		showBalance(savings, checkings);
	}
	return 0;
}

//This function takes in a string and returns 'W', 'D', 'T', 'B', or 'Q' to decide the choice.
MenuOption getChoice(){
	string choice, selection("WDTBQ");

	showMenu();
	cin >> choice;

	while(isMenuChoice(choice)){
		cout << "\nInvalid menu selection.\n";
		showMenu();
		cin >> choice;
	}

	switch (toupper(choice[0])){
		case 'W':
			cout << "\nWithdraw selected.\n";
			return Withdraw;
			break;
		case 'D':
			cout << "\nDeposit selected.\n";
			return Deposit;
			break;
		case 'T':
			cout << "\nTransfer selected.\n";
			return Transfer;
			break;
		case 'B':
			cout << "\nShow balance selected.\n";
			return Balance;
			break;
		case 'Q':
			cout << "\nQuitting.\n";
			return Quit;
			break;
	}
}

//This function returns 'S' or 'C' for savings or checkings.
AccountType selectAccount(){
	string acc;

	showAccountMenu();
	cin >> acc;
	
	while(isAccountChoice(acc)){
		cout << "\nInvalid account selection.\n";
		showAccountMenu();
		cin >> acc;
	}

	switch (toupper(acc[0])){
		case 'C':
			cout << "\nCheckings selected.\n";
			return Checkings;
			break;
		case 'S':
			cout << "\nSavings selected.\n";
			return Savings;
			break;
	}
}

//Assigns a value to amount.
double getAmount(){
	double amount;
	cout << "\nEnter an amount: ";
	cin >> amount;
	return amount;
}

void showMenu(){
	cout << "\n\n[W]ithdraw\n"
		 << "[D]eposit\n"
		 << "[T]ransfer\n"
		 << "Show [B]alance\n"
		 << "[Q]uit\n";
}

void showAccountMenu(){
	cout << "\nAccount Select\n\n"
		 << "[S]avings\n"
		 << "[C]heckings\n";
}

void showBalance(Account sav, Account che){
	cout << "\nSavings: ";
	sav.displayBalance();
	cout << "\nCheckings: ";
	che.displayBalance();
}

bool isMenuChoice(string ch){
	string selection("WDTBQ");

	return selection.find_first_of(toupper(ch[0])) == string::npos;
}

bool isAccountChoice(string ch){
	string selection("SC");

	return selection.find_first_of(toupper(ch[0])) == string::npos;
}


Was This Post Helpful? 0
  • +
  • -

#42 ZenithMkII  Icon User is offline

  • New D.I.C Head

Reputation: 3
  • View blog
  • Posts: 49
  • Joined: 06-September 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 01:26 PM

View Postjimblumberg, on 08 September 2012 - 01:15 PM, said:

Quote

So, find_first_of() returns a size_t datatype. Then, if it != string::npos it returns the size_t number.

Yes find_first_of() returns a size_t datatype, but more importantly it returns std::string::npos if it fails to find the characters in the string. You are comparing this value returned to std::string::npos. You returning the result of this comparison, which is a bool. It may be easier to visualize if you add the parentheses to the return statement:
return(vowels.find_first_of(toupper(ch)) != string::npos);

Or maybe split this statement into it's substeps:

ch = toupper(ch);
size_t position = vowels.find_first_of(ch);
bool found = position != std::string::npos;
return(found);


Jim

Oh, I see. That really clears things up. So, the result of:
position == string::npos
Means that it didn't find anything. Correct?

What is assigned to found if position == string::npos?
-Is it assigned false?
-Or is it really happening like this:
bool found = (position != std::string::npos);


Some kind of argument that returns true of false?
-If it did == it would assign true?
-If it != it would return false?
Was This Post Helpful? 0
  • +
  • -

#43 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3162
  • View blog
  • Posts: 9,541
  • Joined: 05-May 12

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 01:29 PM

The comments in your code are out of date.

Your function names isMenuChoice() and isAccountChoice() is misleading. They return true when it is not one of the choices.

This post has been edited by Skydiver: 08 September 2012 - 01:30 PM

Was This Post Helpful? 1
  • +
  • -

#44 jimblumberg  Icon User is offline

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,735
  • Joined: 25-December 09

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 01:36 PM

Have you checked the logic for this function:
bool isAccountChoice(string ch){
	string selection("SC");

	return selection.find_first_of(toupper(ch[0])) == string::npos;
}


This will return true only if the string doesn't contain an 'S' or a 'C' is that really what you want. This is the same for the other check function. And if you added a parameter, the characters you are checking, you should be able to combine these two functions.

Also how about showing your class files?

Jim
Was This Post Helpful? 1
  • +
  • -

#45 jimblumberg  Icon User is offline

  • member icon


Reputation: 3845
  • View blog
  • Posts: 11,735
  • Joined: 25-December 09

Re: Messy C++ style. Any suggestions?

Posted 08 September 2012 - 02:27 PM

View PostZenithMkII, on 08 September 2012 - 03:26 PM, said:

View Postjimblumberg, on 08 September 2012 - 01:15 PM, said:

Quote

So, find_first_of() returns a size_t datatype. Then, if it != string::npos it returns the size_t number.

Yes find_first_of() returns a size_t datatype, but more importantly it returns std::string::npos if it fails to find the characters in the string. You are comparing this value returned to std::string::npos. You returning the result of this comparison, which is a bool. It may be easier to visualize if you add the parentheses to the return statement:
return(vowels.find_first_of(toupper(ch)) != string::npos);

Or maybe split this statement into it's substeps:

ch = toupper(ch);
size_t position = vowels.find_first_of(ch);
bool found = position != std::string::npos;
return(found);


Jim

Oh, I see. That really clears things up. So, the result of:
position == string::npos
Means that it didn't find anything. Correct?

Correct!

View PostZenithMkII, on 08 September 2012 - 03:26 PM, said:

What is assigned to found if position == string::npos?
-Is it assigned false?
-Or is it really happening like this:
bool found = (position != std::string::npos);


Some kind of argument that returns true of false?
-If it did == it would assign true?
-If it != it would return false?


Don't confuse the relational comparison operators with the result of the comparison. You compare a value with any of the following relational operators "==, != >, <, =<, etc". The result of these comparisons is a bool, and remember a bool has one of two states, false (0), true (not zero). So if you compare two numbers with the operator== the comparison will return true if and only if number1 is equal to number2. If you compare two numbers with the operator!= the comparison will return true if and only if number1 is not equal to number2, and so on.

Jim
Was This Post Helpful? 1
  • +
  • -

  • (5 Pages)
  • +
  • 1
  • 2
  • 3
  • 4
  • 5