Trouble with loops

[cafegeo]

Hello all. This program is supposed to maintain inventory for 5 warehouses with 3 items each warehouse. Each in a different city. You can place an order or send a shipment to each warehouse. When you place an order, if one warehouse does not have enough of the item it searches through other warehouses to fulfill the order. If it finds one to fulfill the order, it takes from that warehouse and adds 10% to the price of the order. If it cannot find a warehouse to fulfill the order it simply says "Order Unfilled". I have the following code and rather than search through other warehouses, it simply says "Order Unfilled", even when other warehouse have enough product to fulfill. I would appreciate any advice, thank you in advance.

switch(ch){
case 1:
	int quantityamount1[3];
	
         for(int i=0;i<3;i++){
         cout<<"Enter quantity for item "<<i+1<<":\n";
	 int quantity=0;
	 cin>>quantity;
	   
           if(quantity<=wh[0].amt[i] && wh[0].amt[i]>0){ //Searching current warehouse inventory
	    wh[0].amt[i]-=quantity;
	      quantityamount1[i]=quantity;
         	     
                     }else{
			for(int j=0;j<5;j++){
                          if(wh[j].amt[i]>0 && quantity<=wh[j].amt[i]){//Searching other warehouses to fulfill order
			  
                          cout<<quantity<<" quantity of item "<<i<<" shipped from "<<wh[j].city<<" to "<<wh[i].city<<"\n";
			  wh[j].amt[i]-=quantity;
			  cout<<wh[j].city<<"  "<<wh[j].amt[0]<<"  "<<wh[j].amt[1]<<"  "<<wh[j].amt[2]<<"\n";
				break;
				  }else{
				    cout<<"Order Unfilled\n";//Could not find any warehouses to fulfill order
		         		 quantityamount1[i]=1;
					break;
					  }
				  }
			  quantityamount1[i]=quantity;
			  }
			}

Here is my warehouse data:

char ch;
	wh[0].city="New York";
	wh[0].amt[0]=40;
	wh[0].amt[1]=33;
	wh[0].amt[2]=43;
	
	wh[1].city="Los Angeles";
	wh[1].amt[0]=25;
	wh[1].amt[1]=23;
	wh[1].amt[2]=26;

	wh[2].city="Miami";
	wh[2].amt[0]=43;
	wh[2].amt[1]=23;
	wh[2].amt[2]=53;

	wh[3].city="Houston";
	wh[3].amt[0]=35;
	wh[3].amt[1]=35;
	wh[3].amt[2]=35;
	
	wh[4].city="Chicago";
	wh[4].amt[0]=21;
	wh[4].amt[1]=21;
	wh[4].amt[2]=21;

[WaltP]

Advice #1: Reformat so the code is readable. Your indenting makes the code unreadable. See this.

[cafegeo]

30 minutes have passed so I could not edit my original. Sorry for the repost. Hope this is more readable.

int quantityamount1[3];
	
        for(int i=0;i<3;i++)
        {
         cout<<"Enter quantity for item "<<i+1<<":\n";
	 int quantity=0;
	 cin>>quantity;
           if(quantity<=wh[0].amt[i] && wh[0].amt[i]>0)  //Searching current warehouse inventory
           { 
	    wh[0].amt[i]-=quantity;
	    quantityamount1[i]=quantity;	     
           }
             else
              {
	        for(int j=0;j<5;j++)
                {
                 if(wh[j].amt[i]>0 && quantity<=wh[j].amt[i])  //Searching other warehouses to fulfill order
		 {
                   cout<<quantity<<" quantity of item "<<i<<" shipped from "<<wh[j].city<<" to "<<wh[i].city<<"\n";
	           wh[j].amt[i]-=quantity;
	           cout<<wh[j].city<<"  "<<wh[j].amt[0]<<"  "<<wh[j].amt[1]<<"  "<<wh[j].amt[2]<<"\n";
			break;
		  }
                    else
                    {
	             cout<<"Order Unfilled\n";//Could not find any warehouses to fulfill order
		     quantityamount1[i]=1;
		     break;
		    }
	         }
		  quantityamount1[i]=quantity;
             }
}

[DavidB]

I took a very quick look through your code, so did not make sure all braces matched up.

However, I think one error might be on line 16:

if(wh[j].amt[i]>0 && quantity<=wh[j].amt[i])

The very first time through the loop, j is 0. But you checked wh[0] in the previous test block; it was the very first warehouse you checked. So this test condition fails the first time through the loop.
So the program jumps to the else block:

cout<<"Order Unfilled\n";
quantityamount1[i]=1;
break;

The program outputs "Order Unfilled" and then breaks out of the loop.

I don't think there is a need to use a break here. You want to cycle through the rest of the wh[j] warehouses don't you? So remove the break, and let the for loop continue going through the rest of the warehouses. You only need a break if you find a non-empty warehouse and then want to avoid searching the rest of the warehouses.

It would probably be best if that block was not in the for loop at all, because it only applies after looping through all the warehouses. Maybe you could use a flag variable to keep track if a warehouse is eventually found, or you could use the j variable.

[cafegeo]

Thank you for the quick reply!

I removed the break and all that does is display "Order unfilled" 5 times consecutively. When removing the else block all together it doesn't seem to look at other warehouses at all.

I even started the "j" for loop at j=1, to skip the first warehouse,wh[0], but no change. It still says order unfilled and never searches the other warehouses.

[WaltP]

30 minutes have passed so I could not edit my original. Sorry for the repost.

No need to apologize. We understand the forum limitations.

Hope this is more readable.

No, it's not.

It actually helps to read and understand the link given, not just make more stuff up.

[cafegeo]

I'm not sure what you mean by "make more stuff up". What exactly am I making up?

I would appreciate the constructive criticism alone, without the senseless, arrogant insult.

I reposted my question below along with the edited code.

Hello all. This program is supposed to maintain inventory for 5 warehouses with 3 items each warehouse. Each in a different city. You can place an order or send a shipment to each warehouse. When you place an order, if one warehouse does not have enough of the item it searches through other warehouses to fulfill the order. If it finds one to fulfill the order, it takes from that warehouse and adds 10% to the price of the order. If it cannot find a warehouse to fulfill the order it simply says "Order Unfilled". I have the following code and rather than search through other warehouses, it simply says "Order Unfilled", even when other warehouse have enough product to fulfill. I would appreciate any advice, thank you in advance.

int quantityamount1[3];
		
for(int i=0;i<3;i++)
{
 cout<<"Enter quantity for item "<<i+1<<":\n";
 int quantity=0;
 cin>>quantity;
   if(quantity<=wh[0].amt[i] && wh[0].amt[i]>0)  //Searching current warehouse inventory
  
    wh[0].amt[i]-=quantity;
    quantityamount1[i]=quantity;
   }
    else
   {
     for(int j=1;j<5;j++)
     {
      if(wh[j].amt[i]>0 && quantity<=wh[j].amt[i])  //Searching other warehouses to fulfill order
      {
       cout<<quantity<<" quantity of item "<<i<<" shipped from "<<wh[j].city<<" to "<<wh[i].city<<"\n";
       wh[j].amt[i]-=quantity;
       cout<<wh[j].city<<"  "<<wh[j].amt[0]<<"  "<<wh[j].amt[1]<<"  "<<wh[j].amt[2]<<"\n";
       break;
      }
       else
       {
	cout<<"Order Unfilled\n";//Could not find any warehouses to fulfill order
	quantityamount1[i]=1;
	break;
       }
     }
	quantityamount1[i]=quantity;
   }
 }

[PrimePackster]

I'm not sure what you mean by "make more stuff up". What exactly am I making up?

I would appreciate the constructive criticism alone, without the senseless, arrogant insult.

Just ignore him.... I think its his style....

And about your code, well you should indent it correctly.....I guess you clearly understand the concepts of pretty printing, each compound statement should have their Braces in same columns, then please but one tab space where ever necessary....

I hope you already know all of this...Anyway repost with the pretty code...

[PrimePackster]

I hope this is how it should be....Still there can be errors, after all its your code, it just formatted it as i think it is should be.....

int quantityamount1[3];
 
        for(int i=0;i<3;i++)
        {
         	cout<<"Enter quantity for item "<<i+1<<":\n";
	 	int quantity=0;
	 	cin>>quantity;
           	if(quantity<=wh[0].amt[i] && wh[0].amt[i]>0)  //Searching current warehouse inventory
           	{ 
	    		wh[0].amt[i]-=quantity;
	    		quantityamount1[i]=quantity;	     
           	}
           	else
           	{
	        	for(int j=0;j<5;j++)
                	{
                 		if(wh[j].amt[i]>0 && quantity<=wh[j].amt[i])  //Searching other warehouses to fulfill order
		 		cout<<quantity<<" quantity of item "<<i<<" shipped from "<<wh[j].city<<" to "<<wh[i].city<<"\n";
	           		wh[j].amt[i]-=quantity;
	           		cout<<wh[j].city<<"  "<<wh[j].amt[0]<<"  "<<wh[j].amt[1]<<"  "<<wh[j].amt[2]<<"\n";
		   		break;
		 	}
                 	else
                 	{
	             		cout<<"Order Unfilled\n";//Could not find any warehouses to fulfill order
		     		quantityamount1[i]=1;
		     		break;
		 	}
	   	}	
		quantityamount1[i]=quantity;
        }
}

Sorry for the repost. Hope this is more readable.

See the difference?

[cafegeo]

I hope this is how it should be....Still there can be errors, after all its your code, it just formatted it as i think it is should be.....

Thank you very much for the edit. It is much better looking this way. I have posted on this forum before and my code never rubbed anyone the wrong way, but I guess all of these For, If and Else loops threw my organizational skills off.

I am hoping people will still take a look at my code and provide some advice.

The algorithm seems precise, but for some reason it is not working as it should.

Thank you again for your understanding patience.

[Schol-R-LEA]

I have noticed that there are some problems with the indented code posted by Captain Jake, specifically that some braces appear to be missing. I've used AStyle to automatically re-indent the code, which I hope will help make the main bug clearer:

{ 
    int quantityamount1[3];

    for(int i=0; i<3; i++)
    {
        cout<<"Enter quantity for item "<<i+1<<":\n";
        int quantity=0;
        cin>>quantity;
        if(quantity<=wh[0].amt[i] && wh[0].amt[i]>0)  //Searching current warehouse inventory

            wh[0].amt[i]-=quantity;
        quantityamount1[i]=quantity;
    }
    else
    {
        for(int j=1; j<5; j++)
        {
            if(wh[j].amt[i]>0 && quantity<=wh[j].amt[i])  //Searching other warehouses to fulfill order
            {
                cout<<quantity<<" quantity of item "<<i<<" shipped from "<<wh[j].city<<" to "<<wh[i].city<<"\n";
                wh[j].amt[i]-=quantity;
                cout<<wh[j].city<<"  "<<wh[j].amt[0]<<"  "<<wh[j].amt[1]<<"  "<<wh[j].amt[2]<<"\n";
                break;
            }
            else
            {
                cout<<"Order Unfilled\n";//Could not find any warehouses to fulfill order
                quantityamount1[i]=1;
                break;
            }
        }
        quantityamount1[i]=quantity;
    }
}

The problem lies in the else statement inside the inner for() loop; the way it is currently situated, it will end the loop prematurely. You want to take the clause out of the loop and use a separate if() statement to check whether the loop was exhausted without finding the item in any of the warehouses:

for(int j=1; j<5; j++)
        {
            if(wh[j].amt[i]>0 && quantity<=wh[j].amt[i])  //Searching other warehouses to fulfill order
            {
                cout<<quantity<<" quantity of item "<<i<<" shipped from "<<wh[j].city<<" to "<<wh[i].city<<"\n";
                wh[j].amt[i]-=quantity;
                cout<<wh[j].city<<"  "<<wh[j].amt[0]<<"  "<<wh[j].amt[1]<<"  "<<wh[j].amt[2]<<"\n";
                break;
            }
        }

        if (j >= 5)  // for loop was exhausted without finding the sought items
        {
            cout<<"Order Unfilled\n";//Could not find any warehouses to fulfill order
            quantityamount1[i]=1;
            break;
        }

As an aside: you might want to use named integer constants for the number of items and warehouses, rather than using a magic number the way you are now. It would make reading and changing the program easier, and it is a good habit to get into.

[WaltP]

I'm not sure what you mean by "make more stuff up". What exactly am I making up?

I gave you a link to explain how to format your code. You ignored it and reformatted in a completely useless way. I posted the link for you to learn from.

I would appreciate the constructive criticism alone, without the senseless, arrogant insult.

It wasn't an insult. And I gave you constructive criticism and you ignored the help.

And about your code, well you should indent it correctly.....

Gee, isn't this exactly what I said? And at least I included a link to explain how! Your comment didn't help at all... But then, I notice, that's your way.

[PrimePackster]

Gee, isn't this exactly what I said? And at least I included a link to explain how! Your comment didn't help at all... But then, I notice, that's your way.

When people here in forum can't understand what you are saying, it's your problem. Please make your points more clear....Don't make it feel like a insult.....:P

[WaltP]

When people here in forum can't understand what you are saying, it's your problem. Please make your points more clear....Don't make it feel like a insult.....:P

You're right.
Here's exactly what I said:

Advice #1: Reformat so the code is readable. Your indenting makes the code unreadable. See this.

How can I make it clearer? Please teach me...

[Schol-R-LEA]

The link which WaltP provided is to a page for precisely that, actually; this link here gives details on different approaches to indent style, with a specific style (Allman) highlighted. A broad search on indent style or brace style should turn up a plethora of information.

If all else fails, you might also look into editing your code with an IDE that has an auto-indent feature. I have lately been using Code::Blocks a lot lately, and that comes with the Astyle tool which I'd used earlier (under Plugins). Visual C++ Express has a good auto-styler as well.

What editor or IDE are you using? Chances are, it already has support for auto-indentation, even if you don't know it.

[PrimePackster]

I have noticed that there are some problems with the indented code posted by Captain Jake

Thanks for the info...Just as i expected, WEll Thank You. Your code can really do the JOb for OP