Trouble with loops

Question

cafegeo 10 Light Poster

12 Years Ago

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;

c++

5 Contributors
15 Replies
155 Views
3 Days Discussion Span
Latest Post 12 Years Ago Latest Post by PrimePackster

All 15 Replies

WaltP 2,905 Posting Sage w/ dash of thyme

12 Years Ago

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

DavidB 44 Junior Poster

12 Years Ago

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.

Schol-R-LEA 1,446 Commie Mutant Traitor

12 Years Ago

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.

Edited 12 Years Ago by Schol-R-LEA because: n/a

PrimePackster commented: Thanks for pointing out +0

Schol-R-LEA 1,446 Commie Mutant Traitor

12 Years Ago

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.

Edited 12 Years Ago by Schol-R-LEA because: n/a

Reply to this topic

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.

cafegeo 10 Light Poster · Answer 1 · 2011-12-02T00:50:40+00:00

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;
             }
}

cafegeo 10 Light Poster · Answer 2 · 2011-12-02T02:01:11+00:00

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 2,905 Posting Sage w/ dash of thyme Team Colleague · Answer 3 · 2011-12-02T03:00:35+00:00

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 10 Light Poster · Answer 4 · 2011-12-03T10:15:52+00:00

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 10 Runaway Poster · Answer 5 · 2011-12-03T19:08:07+00:00

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 10 Runaway Poster · Answer 6 · 2011-12-03T19:18:59+00:00

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 10 Light Poster · Answer 7 · 2011-12-03T23:09:38+00:00

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.

WaltP 2,905 Posting Sage w/ dash of thyme Team Colleague · Answer 8 · 2011-12-04T06:06:13+00:00

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 10 Runaway Poster · Answer 9 · 2011-12-04T16:48:30+00:00

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 2,905 Posting Sage w/ dash of thyme Team Colleague · Answer 10 · 2011-12-05T02:11:25+00:00

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...

PrimePackster 10 Runaway Poster · Answer 11 · 2011-12-05T10:10:41+00:00

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

Trouble with loops

Recommended Answers Collapse Answers

All 15 Replies

Recommended Answers