If Else If

[OleFowdie]

Ok this program is supposed to be a POS program in a paint shop. There's a section where I am using if else if selection/branching structures and the program is dumping everything into "other" on the end of day report screen before program termination.

#include <iostream>
	using namespace std;
#include <iomanip>
#include <cmath> 

void init(void);
void splash(void);
void getInput(void);
void doCalc(void);
void sendOutput(void);
void loop(void);
void eodr(void);

double fHeight, fWidth, fLength, fPaintSpace, fGallonCost, fGallonsNeeded, fArea;
char sFirstName[16], sLastName[16], sState[3], cSalesDay, sFirstHigh[16], sLastHigh[16];
double fCost, fCostTax, fCostDiscount, fRedundant;
double CountGA, CountFL, CountSC, CountTN, CountAL, CostGA, CostFL, CostSC, CostTN, CostAL, TotalCount, TotalCost, CountOther, CostOther;

int main() {
	init();
	splash();
	loop();
	eodr();
	return 0;
}

void loop(void) {
  while ((cSalesDay == 'y') || (cSalesDay == 'Y')) {
		system("cls");
		getInput();
		doCalc();
		sendOutput();
	    splash();
		system("cls");
  }
return;}

void splash(void) {
	cout << "********************************************************\n";
    cout << "          Acme Paint Shop - Sales day question          \n";
	cout << "********************************************************\n\n";
	cout << "Is today a sales day (Y/N)? ";
	cin >> cSalesDay;
	system("cls");
return;}
	
void getInput(void) {
	cout << "********************************************************\n";
    cout << "              Acme Paint Shop - Input                   \n";
	cout << "********************************************************\n\n";
	cout << "Customer Name (first and last):  ";
	cin >> sFirstName >> sLastName;
	cout << "\n\nHeight:           "; // get the height of the room to be painted
	cin >> fHeight;
	cout << "\n\nWidth:            "; // get width
	cin >> fWidth;
	cout << "\n\nLength:           "; // get length
	cin >> fLength;
	cout << "\n\nGallon Coverage:  ";  // get fPaintSpace
	cin >> fPaintSpace;
	cout << "\n\nCost Per Gallon:  "; // get gallon cost
	cin >> fGallonCost;
    cout << "\n\nState:            ";
	cin >> sState;

	system("cls");

	if ((fGallonCost >= 1) && (fGallonCost <= 50)) {
		return;}
	else {
		cout << "\n\nInvalid. Please enter a number between 1 and 50: ";
		cin >> fGallonCost;
		if ((fGallonCost >= 1) && (fGallonCost <= 50)) {
				system("cls");
				return;}
		else {
				cout << "\nInvalid!!";
				system("cls");
				return;}
	}
}

void doCalc(void) {

/* Customer Sale Calculation */

	fArea = fHeight * fWidth; // get one wall's area
	fGallonsNeeded = fHeight * fLength; // get perpendicular wall's area
	fArea *= 2; // account for two walls
	fGallonsNeeded *= 2; // account for two walls
	fArea = fArea + fGallonsNeeded; // add for total area to be painted
	fGallonsNeeded = ceil (fArea / fPaintSpace); // get number of gallons needed
	fCost = fGallonsNeeded * fGallonCost; // calculate price

/* State Gallon Counter */

	if ((sState == "ga") || (sState == "GA") || (sState == "gA") || (sState == "Ga")){ 
		CountGA = CountGA + fGallonsNeeded;}
	else if ((sState == "fl") || (sState == "FL") || (sState == "fL") || (sState == "Fl")){
		CountFL = CountFL + fGallonsNeeded;}
	else if ((sState == "sc") || (sState == "SC") || (sState == "sC") || (sState == "Sc")){
		CountSC = CountSC + fGallonsNeeded;}
	else if ((sState == "tn") || (sState == "TN") || (sState == "tN") || (sState == "Tn")){
		CountTN = CountTN + fGallonsNeeded;}
	else if ((sState == "al") || (sState == "AL") || (sState == "aL") || (sState == "Al")){
		CountAL = CountAL + fGallonsNeeded;}
	else{CountOther = CountOther + fGallonsNeeded;}

/* Discount Calculations */

	if (fGallonsNeeded == 1){fCostDiscount = fCost * 0.95;}
	else if ((fGallonsNeeded == 2) || (fGallonsNeeded == 3)){fCostDiscount = fCost * 0.90;}
	else if (fGallonsNeeded == 4){fCostDiscount = fCost * 0.85;}
	else{fCostDiscount = fCost * 0.80;}

/* STARTING THE TAX SECTION */

	if ((sState == "ga") || (sState == "GA") || (sState == "gA") || (sState == "Ga")){
		fCostTax = fCostDiscount * 1.06;}
	else if ((sState == "fl") || (sState == "FL") || (sState == "fL") || (sState == "Fl")){
		fCostTax = fCostDiscount * 1.04;}
	else if ((sState == "sc") || (sState == "SC") || (sState == "sC") || (sState == "Sc")){
		fCostTax = fCostDiscount * 1.05;}
	else if ((sState == "tn") || (sState == "TN") || (sState == "tN") || (sState == "Tn")){
		fCostTax = fCostDiscount * 1.05;}
	else if ((sState == "al") || (sState == "AL") || (sState == "aL") || (sState == "Al")){
		fCostTax = fCostDiscount * 1.02;}
	else{fCostTax = fCostDiscount * 1.00;}

/* Cost by State Counter */

	if ((sState == "ga") || (sState == "GA") || (sState == "gA") || (sState == "Ga")){
		CostGA = CostGA + fCostTax;}
	else if ((sState == "fl") || (sState == "FL") || (sState == "fL") || (sState == "Fl")){
		CostFL = CostFL + fCostTax;}
	else if ((sState == "sc") || (sState == "SC") || (sState == "sC") || (sState == "Sc")){
		CostSC = CostSC + fCostTax;}
	else if ((sState == "tn") || (sState == "TN") || (sState == "tN") || (sState == "Tn")){
		CostTN = CostTN + fCostTax;}
	else if ((sState == "al") || (sState == "AL") || (sState == "aL") || (sState == "Al")){
		CostAL = CostAL + fCostTax;}
	else{CostOther = CostOther + fCostTax;}

/* Keeping track of Totals */

	TotalCount += fGallonsNeeded;
	TotalCost += fCostTax;

	if (fCostTax > fRedundant){
		fRedundant = fCostTax;
		strcpy_s(sFirstHigh,sFirstName);
		strcpy_s(sLastHigh,sLastName);}

return;} // End doCalc()
	
void sendOutput(void) {
	cout << "********************************************************\n";
    cout << "      Acme Paint Shop - Single Customer Calculation     \n";
	cout << "********************************************************\n\n";
	cout << "Customer Name:            " << sFirstName << " " << sLastName << endl;
	cout << "\nNumber of Gallons Needed: " << fGallonsNeeded << endl;
	cout << "\nCost Before Discount:     $" << setprecision(2) << fixed << fCost << endl;
	cout << "\nCost After Discount:      $" << setprecision(2) << fixed << fCostDiscount << endl;
	cout << "\nCost With Tax:            $" << setprecision(2) << fixed << fCostTax << endl;
return;}

void eodr(void) {
	cout << "********************************************************\n";
    cout << "         Acme Paint Shop - End of Day Report            \n";
	cout << "********************************************************\n\n";
	cout << "State      # of Cans      Total Cost After Discount/Tax\n";
	cout << "GA           " << setprecision(0) << fixed << CountGA << "                     $" << CostGA << endl;
	cout << "FL           " << setprecision(0) << fixed << CountFL << "                     $" << CostFL << endl;
	cout << "SC           " << setprecision(0) << fixed << CountSC << "                     $" << CostSC << endl;
	cout << "TN           " << setprecision(0) << fixed << CountTN << "                     $" << CostTN << endl;
	cout << "AL           " << setprecision(0) << fixed << CountAL << "                     $" << CostAL << endl;
	cout << "Other        " << setprecision(0) << fixed << CountOther << "                     $" << CostOther << endl;
	cout << "________________________________________________________\n";
    cout << "Grand Total  " << TotalCount << "                     $" << TotalCost << endl;
	cout << "\nCustomer With Highest Total After Tax: " << sFirstHigh << sLastHigh << endl;
	cout << "Customer Total:                         $" << setprecision(2) << fixed << fRedundant << endl;
	return;
}
void init(void) {

	fHeight = 0.0; fWidth = 0.0; fLength = 0.0; fPaintSpace = 0.0; fGallonCost = 0.0;
	fGallonsNeeded = 0.0; fArea = 0.0; fCost = 0.0;
	fCostTax = 0.0; fCostDiscount = 0.0; fRedundant = 0.0; CountGA = 0.0; CountFL = 0.0;
	CountSC = 0.0; CountTN = 0.0; CountAL = 0.0; CostGA = 0.0; CostFL = 0.0; CostSC = 0.0;
	CostTN = 0.0; CostAL = 0.0; TotalCount = 0.0; TotalCost = 0.0; CountOther = 0.0; CostOther = 0.0;
}

[Zaita]

Firstly, you need to fix your syntax. Your If-Else statements are almost un-readable. Placing the closing bracket at the end of a statement is not any known coding-standard I am aware of.

return;} = BAD

return;
}

= GOOD

Good Example:

if ((fGallonCost >= 1) && (fGallonCost <= 50)) {
  return;
} else {
  cout << "\n\nInvalid. Please enter a number between 1 and 50: ";
  cin >> fGallonCost;
  if ((fGallonCost >= 1) && (fGallonCost <= 50)) {
    system("cls");
    return;
  } else {
    cout << "\nInvalid!!";
    system("cls");
    return;
  }
}

Some other issues to point out.
char sFirstName[16], sLastName[16], sState[3], cSalesDay, sFirstHigh[16], sLastHigh[16];

All of these are fixed length. Yet at no point do you verify the length of input before putting it into these. If I enter a 5 letter state code, that should constitute a buffer overflow bug in your application.

The next issue, is that you are not giving any default values to the above character arrays. Therefore when doing a check against sState == "ga" this can never be true because sState[2] == UNDEFINED. You need to set it to a proper value before use.

I would also suggest once you have received your sState that you iterate through and use the toLower() function on it. This would save you having such large if (sState == "XX") functions.

strcpy_s is not part of the ANSI C++ (that I am aware). For a safe strcpy function you should use strncpy().

As a professional developer. I HIGHLY HIGHLY suggest that you use the string datatype instead of character arrays. They have built in allocation so you will not have to worry about buffer overflows etc.

e.g

#include <iostream>
#include <string>

using namespace std;

int main() {

  string sState = "";

  cout << "Please enter state: ";
  cin >> sState;

  for (unsigned int i = 0; i < sState.length(); ++i)
    sState[i] = tolower(sState[i]);
 
  if (sState == "ga") // Notice, works with Ga, gA, GA and ga
    cout << "You are in GA" << endl;
  else
    cout << "I am sorry, I do not know: " << sState << endl;

  return 0;
}

[OleFowdie]

Thanx for the tip, but C++ is not space/indent sensitive like Python or anything else. I use a method that works for me. I can change my code for posts, but in my actual source code documents on my machine won't change ;)

[guestgulkan]

Ah I see - the problem is that sState is an array of chars .
So for example in line 97 it is legal for you to say if ((sState == "ga") BUT YOU ARE DOING THE WRONG THING, because that means that you are comparing the ADDRESS of the array not it's contents, that is why the tests fails and all the stuff ends up in the 'other' bin.

You need to use a string COMPARISON function.
You can use the C function strcmp or as you are using c++ use one of the string functions to do this.

[Zaita]

but C++ is not space/indent sensitive like Python or anything else.

Correct. But it is important (VERY) for your code to be maintainable. Even for assignments it's a good habbit to get into. If you presented that code to me in a portfolio for a job here; I would turn you down on the presentation of your code alone. That code to me, would be extremely difficult to maintain.

You have to be aware that other people will be expected to work on your code should something happen to you. It's important to write code in a consistent manner that conforms to a known/common coding standard. (and by looking at your code, your convention is Hungarian notation.)

The layout I posted above is the (Very common) K&R Method.

but C++ is not space/indent sensitive like Python or anything else.

That's like saying "I don't need to wear a seatbelt, because the car runs without it plugged in". Sure it's true, but definitely not the best course of action.

[Zaita]

@guestgulkan: Correct :)
Here is my sample modified to use a char array. Again, it's not safe because cin (AFAIK) doesn't check input length before assigning it.

#include <iostream>
#include <string>

using namespace std;

int main() {

  char sState[4] = {0};

  cout << "Please enter state: ";
  cin >> sState;

  for (unsigned int i = 0; i < strlen(sState); ++i)
    sState[i] = tolower(sState[i]);

  if (strcmp(sState,"ga") == 0) // Notice, works with Ga, gA, GA and ga
    cout << "You are in GA" << endl;
  else
    cout << "I am sorry, I do not know: " << sState << endl;

  return 0;
}

[OleFowdie]

Thank you both! I have been puzzling over this problem for quite a while and couldn't figure it out.

Zaita - I understand what you are saying, but if I comment everything I ought to be ok, right?

(I had to remove a lot of the commenting to make the program code fit into the posting area)

[Zaita]

but if I comment everything I ought to be ok, right?

Nope. There is alot of fanboi-ism over different coding styles and namng conventions. Showing your ability to follow a standardized one correctly is definitely going to help you professionally.

Comments are extremely important. Just don't over-comment also.

You really should stick to 1 of the 2 major layout conventions for braces though.

Way 1:

if (something == true)
{
 function();
}
else
{ 
 function();
}

Way 2

if (something == true) {
 function();
} else {
 function();
}

Now, take your style and a few indentations and this is what we'd end up with.

int main() {
  while (true) {
    if (something == true) {
      callFunction();
      callOtherFunction();
      if (something == false)
        callFunction();}
    else {
      callSomeOtherFunction();
      if (bob == name)
        renameBob();}
    callCleanUp();}
  
  return 0;
}

That is almost un-followable without analysing each line individually. You have no ability to isolate blocks of logic mentally and determine the pattern of flow.

Only because my IDE has strict Indentation does it actually help.

[jsmith]

IMHO, comments should explain why the code is doing what it is doing, not what it is doing. If it becomes necessary to explain what the code is doing because it is not obvious, then you should consider rewriting it to simplify. As Zaita points out, maintainability in the professional environment is very important --not only for others to understand what your code does, but also to help you remember what your code does months or years after you originally wrote it. (Take it from someone who has firsthand experience with that... code I wrote a couple of years ago seemed so simple at the time is much harder to understand once I've forgotten all the fine details).

[Faldrax]

Personally I find that it can be helpful to have comments for both what and why, but better still is to write code that keeps use fo comments to a minimum through careful choice of names for classes, methods, varaibles, etc.
Comments can also be used to help section your code - but this is more relavant to functional rather than OO design.

[toshiro]

Zaita, a quick question: Dev-C++ seems to use its own way, namely:

int test()
{
     //stuff
     if()
     {
          //more stuff
          }
          else
          {
               //even more stuff
               }
               }

This is horrible to read, and I, myself, use the first way you described. I just thought it odd that an auto-indentation feature in an IDE should behave that badly (it even says it will indent to the first non-whitespace char of the preceding line).

It just led me to turning off auto-indentation :/

[mikeb570]

I don't know what Dev-C++ you're using but mine seems to work fine. For me, lines 7, 11, and 12 would have gone to the right places. More precisely when you press } it jumps back a tab. I have smart tabs off though which is on by default, so maybe that's why.

Edit, I just tried it and with smart tabs and auto indent on it acts funny. But works fine with smart tabs off and auto indent on. Anyways it's not perfect, but if it saves a keypress 85% of the time I think it's worth it.

[toshiro]

Hmm, I did what you suggested. Thanks a bunch, it works fine now, as you said... but I'm not going to check the rest of my code (half of which is neither written by me, nor in C++, but C).