Feedback on my programming style

[dydx31]

Here is a simple program that I wrote:

//This program takes in various inputs.
//It calculates if the current balance exceeds the credit limit.
//Programmer: Nathan D. Brown
//Date: 11/14/06

#include<iostream>
#include<iomanip>
using std::cout;
using std::cin;
using std::endl;

int main()
{
int account_num;
double beg_bal;
double tot_charges;
double tot_credits;
double credit_limit;
double new_balance;

while(1){
cout<<"Please enter account number-1 to end)"<<endl;
cin>>account_num;
if(account_num == -1)
exit(1);
else{
cout<<"Enter beginning bal:"<<endl;
cin>>beg_bal;
cout<<"Enter total charges:"<<endl;
cin>>tot_charges;
cout<<"Enter total credits:"<<endl;
cin>>tot_credits;
cout<<"Enter credit limit:"<<endl;
cin>>credit_limit;
new_balance = beg_bal + tot_charges -
tot_credits;
}

if(new_balance > credit_limit){
cout<<"Acct number: "<<account_num<<endl;
cout<<"Credit limit: "<<credit_limit<<endl;
cout<<"New Balance: "<<new_balance<<endl;
cout<<"Credit limit exceeded."<<endl;
}

}
}

--------------------------------------------------------------------
--------------------------------------------------------------------
-------------------

I need some simple feedback: How is my programming style? Can I
improve anything in the code for better performance etc?
ANY FEEDBACK YOU PROVIDE WILL BE GREATLY APPRECIATED!!!! I plan on
making programming into my career.

Thanks again,,

Nathan

 

[Victor Bazarov]

Here is a simple program that I wrote:

//This program takes in various inputs.
//It calculates if the current balance exceeds the credit limit.
//Programmer: Nathan D. Brown
//Date: 11/14/06

#include<iostream>
#include<iomanip>
using std::cout;
using std::cin;
using std::endl;

int main()
{
int account_num;
double beg_bal;
double tot_charges;
double tot_credits;
double credit_limit;
double new_balance;

while(1){
cout<<"Please enter account number-1 to end)"<<endl;
cin>>account_num;
if(account_num == -1)
exit(1);
else{
cout<<"Enter beginning bal:"<<endl;
cin>>beg_bal;
cout<<"Enter total charges:"<<endl;
cin>>tot_charges;
cout<<"Enter total credits:"<<endl;
cin>>tot_credits;
cout<<"Enter credit limit:"<<endl;
cin>>credit_limit;
new_balance = beg_bal + tot_charges -
tot_credits;
}

if(new_balance > credit_limit){
cout<<"Acct number: "<<account_num<<endl;
cout<<"Credit limit: "<<credit_limit<<endl;
cout<<"New Balance: "<<new_balance<<endl;
cout<<"Credit limit exceeded."<<endl;
}

}
}

--------------------------------------------------------------------
--------------------------------------------------------------------
-------------------

I need some simple feedback: How is my programming style? Can I
improve anything in the code for better performance etc?
ANY FEEDBACK YOU PROVIDE WILL BE GREATLY APPRECIATED!!!! I plan on
making programming into my career.

First of all, do not declare any local variables before you need them.
Second, initialise all your variables (at least in the beginning of
your career).

Having defined a branch that causes 'exit' (which is generally a bad
idea, it's better to return gracefully from any function, even 'main'),
you don't need to specify 'else', the unnecessary block causes
visibility confusion.

None of your input is checked for validity. What if I type 'abc' where
you expect a number? It may be better to define a "read a number" kind
of function for entering all those numbers.

Only one of your prompts specifies what value range is valid. Are
negative numbers accepted?

There is nothing in your program that requires or has any opportunity
for performance improvement, it's a UI program (most time is spent
waiting for user input) and there are very few calculations.

To make your code more readable:
Use spaces instead of tabs.
Surround binary operators (except dot and arrow) with spaces.

V

 

[E. Robert Tisdale]

Here is a simple program that I wrote:

#include<iostream>
#include<iomanip>

int main(int argc, char* argv[]) {

while (1) {
int account_num = -1;
std::cout << "Please enter account number-1 to end)" << std::endl;
std::cin >> account_num;
if (account_num == -1) {
break;
}
else {
double beg_bal = 0;
std::cout << "Enter beginning bal: " << std::endl;
std::cin >> beg_bal;
double tot_charges = 0;
std::cout << "Enter total charges: " << std::endl;
std::cin >> tot_charges;
double tot_credits = 0;
std::cout << "Enter total credits: " << std::endl;
std::cin >> tot_credits;
double credit_limit = 0;
std::cout << "Enter credit limit: " << std::endl;
std::cin >> credit_limit = 0;
double new_balance = beg_bal + tot_charges - tot_credits;

if(new_balance > credit_limit) {
std::cout << " Acct number: " << account_num << std::endl;
std::cout << "Credit limit: " << credit_limit << std::endl;
std::cout << " New Balance: " << new_balance << std::endl;
std::cout << "Credit limit exceeded." << std::endl;
}
}
}
return 0;
}

I need some simple feedback: How is my programming style?
Can I improve anything in the code for better performance etc?

I don't think that your version worked the way that it was supposed to
work. Try mine instead.

 

[dydx31]

Actually ,

My program works perfectly fine. I'm using microsoft visual studio
2005 to compile the code. Sorry if Im kind of new,,but im a novice
programmer. I'm using the Deitel C++ how to program book. They
didnt mention any other methods than the ones i used.

Thanks for your responses!

Nathan

 

[dasjotre]

To add:

Minimize use of endl. endl flushes the stream which
in the case of

cout<<"Enter beginning bal:"<<endl;
cin>>beg_bal;

is not necessary since the first line must
appear on the screen before second line is
executed

cout<<"Enter beginning bal:\n";
cin>>beg_bal;

works just the same.

cout<<"Acct number: "<<account_num<<endl;
cout<<"Credit limit: "<<credit_limit<<endl;

can easily be changed to:

cout <<"Acct number: "<<account_num
<<"\nCredit limit: "<<credit_limit
<< endl;

typing '=' when you mean '==' is a common mistake
since if(account_num = -1) is well formed, a compiler
might not even issue a warning

prefer if(1- == account_num)
to if(account_num == -1)

so the compiler complains if you mistype

 

[razael1]

Instead of a while(1) loop that calls exit(), it's probably better to
do something like this:

cout<<"Please enter account number-1 to end)"<<endl;
cin>>account_num;

while(account_num != -1) {
...
cout<<"Please enter account number-1 to end)"<<endl;
cin>>account_num;
}


While this removes your exit(1) call, if you stayed with the above
program, you would probably want exit(0). Your program returning 0 is
interpreted by most (all?) operating systems as meaning your program
executed successfully. By returning 1, you are semantically indicating
that your program exited because it encountered an error, which is not
true. This is important if someone is running your program in a script
they are writing. They will want to check the exit code to make sure
it did what was expected, and it's assumed you will return 0 if it did.

Colin K.

 

[Noah Roberts]

I need some simple feedback: How is my programming style? Can I
improve anything in the code for better performance etc?
ANY FEEDBACK YOU PROVIDE WILL BE GREATLY APPRECIATED!!!! I plan on
making programming into my career.

Your function is kind of long. Good candidate for refactoring into
smaller ones. Google refactoring.

 

[Yannick Tremblay]

To add:

Minimize use of endl. endl flushes the stream which
in the case of

cout<<"Enter beginning bal:"<<endl;
cin>>beg_bal;

is not necessary since the first line must
appear on the screen before second line is
executed

cout<<"Enter beginning bal:\n";
cin>>beg_bal;

works just the same.

Potentially gain 0.01 sec in execution time
for a user interface module.

So I fail to see the point.

Flushing the stream can have advantages if execution
speed is not the first consideration. (think about
a multi-process application)

cout<<"Acct number: "<<account_num<<endl;
cout<<"Credit limit: "<<credit_limit<<endl;

can easily be changed to:

cout <<"Acct number: "<<account_num
<<"\nCredit limit: "<<credit_limit
<< endl;

To me, the original version is more readable
and more maintainable but that's probably a matter
of taste.

typing '=' when you mean '==' is a common mistake
since if(account_num = -1) is well formed, a compiler
might not even issue a warning

prefer if(1- == account_num)
to if(account_num == -1)

so the compiler complains if you mistype

Agree with that one.

 

[razael1]

prefer if(1- == account_num)
to if(account_num == -1)

so the compiler complains if you mistype

Most people will notice the typo, but just in case:
if(-1 == account_num)
not
if(1- == account_num)

Colin K.

 

[Jim Langston]

Here is a simple program that I wrote:

//This program takes in various inputs.
//It calculates if the current balance exceeds the credit limit.
//Programmer: Nathan D. Brown
//Date: 11/14/06

#include<iostream>
#include<iomanip>
using std::cout;
using std::cin;
using std::endl;

int main()
{
int account_num;

Lets initialize this:
int account_num = 0;

double beg_bal;
double tot_charges;
double tot_credits;
double credit_limit;
double new_balance;

while(1){

Do forever loops are ugly. I would prefer:
while ( account_num != -1 )
{

cout<<"Please enter account number-1 to end)"<<endl;
cin>>account_num;
if(account_num == -1)
exit(1);

Exit is ugly, and with our while loop changed this would become:

if ( account_num == -1 )
continue;

else{

No need for the else. Even with your exit it wouldn't get here if the if
was true. So remove it.

cout<<"Enter beginning bal:"<<endl;
cin>>beg_bal;

What if they typed in a bal of "x"? beg_bal now has an undefined value (I
believe it's left to it's orignal value) and worst, our cin is now in a bad
state. For non trivial programs you need to check the result. Simplist
being:

if ( cin >> beg_bal )

returns true if it took a value, is false if it couldn't read the input.
Not sure if

if ( ! cin >> beg_bal )
works or not. I'll have to test.

cout<<"Enter total charges:"<<endl;
cin>>tot_charges;
cout<<"Enter total credits:"<<endl;
cin>>tot_credits;
cout<<"Enter credit limit:"<<endl;
cin>>credit_limit;
new_balance = beg_bal + tot_charges -
tot_credits;
}

if(new_balance > credit_limit){
cout<<"Acct number: "<<account_num<<endl;
cout<<"Credit limit: "<<credit_limit<<endl;
cout<<"New Balance: "<<new_balance<<endl;
cout<<"Credit limit exceeded."<<endl;
}

}
}

--------------------------------------------------------------------
--------------------------------------------------------------------
-------------------

I need some simple feedback: How is my programming style? Can I
improve anything in the code for better performance etc?
ANY FEEDBACK YOU PROVIDE WILL BE GREATLY APPRECIATED!!!! I plan on
making programming into my career.

One thing I feel that changes a good programmer from a great programmer, is
error checking. Check *ALL* user input. Anything the user can input can
( and will ) be bad. It tends to make a simple one line like:
cin >> account_bal;
into 5 or 6 lines, but your program becomes more robust and doesn't break.
You have to think, what if they typed in somethign that wasn't a number?
What do I want to do? Maybe I should just put it in a loop:

while ( ! cin >> account_bal )
{
std::cout << "Bad input, please enter a value for account balance, -1 to
exit\n";
cin.clear(); // More may be needed than this
}

Over the years I've discovered that if you make an idiot proof program, they
come out with a better idiot, but you can do the best you can.

 

[Frederick Gotham]

Yannick Tremblay:

Potentially gain 0.01 sec in execution time
for a user interface module.

An entire 10 million nanoseconds!