Problem in for loop condition using sizeof()

hey i made a program that calculates bills

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
class television                                                                         //TELEVISION CLASS
{
protected:
    char allTVs[5][15]={"Samsung" , "Sony", "LG", "Panasonic", "Onida"};
    float priceTVs[5]={75000.00,80000.00,120000.00,50000.00,35000.00};
public:
    void viewTVs()                               //VIEWS ALL TVS
    {
        for(int i=0; i<5; i++)
            cout<<"\n"<<i+1<<" "<<allTVs[i]<<setw(20)<<setprecision(8)<<priceTVs[i];
    }
};

class options: public laptops, mobiles, television                                       //OPTIONS CLASS
{
private:
    int category;
    int x=0,y=0,z=0;
    float totalPrice=0;
    int productID;
    int check;
    int lap[10];
    int mob[10];
    int tv[10];
    int k;
    int checkLap;
    int checkMob;
    int checkTV;
    int i;
    int bill=0;
    int tempBill;
    int l;
    struct bill
    {
        int billNo=0;
        string fname,lname;
        int cost=0;
    }bObj[50];
public:
    void setDefault()                            //SETDEFAULT :SETS ALL VALUES TO ZERO 
    {
        check=0;
        for(i=0;i<10;i++)
        {
            lap[i]=-1;
            mob[i]=-1;
            tv[i]=-1;
        }
        checkLap=0;
        checkMob=0;
        checkTV=0;
        totalPrice=0;
        x=0;
        y=0;
        z=0;
    }
    void enterItems()                            //ENTERITEMS :ALLOWS USER TO ENTER ITEMS
    {
        enterBegin:
        productID=1;
        cout<<"\n Enter category:";
        cout<<"\n 0.Exit \n 1.Laptops \n 2.Mobiles \n 3.Televisions \n 4.Proceed to bill"<<endl;
        A:
        cin>>category;
        switch(category)
        {
        case 0:
            check=1;
            break;
        case 1:         //LAPTOPS
            system("cls");
            viewLaptops();
            while(productID!=0)
            {
                cout<<"\n Enter product number (0 to quit): ";
                L:
                cin>>productID;
                if(productID==0)
                    break;
                else if(productID>5 || productID<1)
                {
                     cout<<"\n No such product please try again :";
                     goto L;
                }
                else
                {
                    checkLap=1;
                    totalPrice+=priceLaptops[productID-1];
                    cout<<"\n \t \t Price :"<<priceLaptops[productID-1];
                    lap[x]=productID;
                    ++x;
                }
            }
            break;
        case 2:         //MOBILES
            system("cls");
            viewMobiles();
            while(productID!=0)
            {
                cout<<"\n Enter product number (0 to quit): ";
                M:
                cin>>productID;
                if(productID==0)
                    break;
                else if(productID>5 || productID<1)
                {
                     cout<<"\n No such product please try again :";
                     goto M;
                }
                else
                {
                    checkMob=1;
                    totalPrice+=priceMobiles[productID-1];
                    cout<<"\n \t \t Price :"<<priceMobiles[productID-1];
                    mob[y]=productID;
                    ++y;
                }
            }
            break;
        case 3:         //TVS
            system("cls");
            viewTVs();
            while(productID!=0)
            {
                cout<<"\n Enter product number (0 to quit): ";
                T:
                cin>>productID;
                if(productID==0)
                    break;
                else if(productID>5 || productID<1)
                {
                     cout<<"\n No such product please try again :";
                     goto T;
                }
                else
                {
                    checkTV=1;
                    totalPrice+=priceTVs[productID-1];
                    cout<<"\n \t \t Price :"<<priceTVs[productID-1];
                    tv[z]=productID;
                    ++z;
                }
            }
            break;
        case 4:
            printBill();
            break;
        default:
            cout<<"\n Enter a number from 0 to 4 :";
            goto A;
        }
        if(check!=1)
            goto enterBegin;
    }
    void viewAll()                               //VIEWALL :VIEWS ALL PRODUCTS BY INVOKING MEMBER FUNCTIONS OF INHERITED CLASS
    {
        cout<<"PRODUCT"<<setw(20)<<"PRICE";
        cout<<"\n Laptops\n";
        viewLaptops();
        cout<<"\n Mobiles\n";
        viewMobiles();
        cout<<"\n Televisions\n";
        viewTVs();
        cout<<endl;
        cout<<"\n Enter any key to continue :";
        getch();
    }
    void assignID()
    {
        srand(time(0));
        cout<<"\n Enter first name of purchaser :";
        cin>>bObj[bill].fname;
        cout<<"\n Enter last name of purchaser :";
        cin>>bObj[bill].lname;
        bObj[bill].cost=totalPrice;
        bObj[bill].billNo=(rand()%99999)+10000;
        ++bill;
    }
    void viewID()
    {
        if(bill>0)
        {
            tempBill=bill;
            for(bill=0; bill<tempBill; bill++)
            {
                if(bObj[bill].cost==0)
                    break;
                cout<<"\n CUSTOMER NUMBER "<<bill+1;
                cout<<"\n Bill no.       \t:"<<bObj[bill].billNo;
                cout<<"\n Customers name \t:"<<bObj[bill].fname<<' '<<bObj[bill].lname;
                cout<<"\n Total bill     \t:"<<bObj[bill].cost<<endl;
            }
        bill=tempBill;
        }
        else
            cout<<"\n No Sales Recorded"<<endl;
        cout<<"\n Enter any key to continue :";
        getch();
    }
    void printBill()                             //PRINTBILL :PRINTS OUT ALL PRODUCTS BOUGHT AND THEIR RESPECTIVE PRICE
    {
        system("cls");
        cout<<"\n  Products Bought \t \t \t Price"<<endl;
        if(checkLap==1) //LAPTOP CHECK
        {
            cout<<"\n Laptops"<<endl;
            for(x=0;lap[x]<=5;x++)
            {
                if((lap[x]-1)<0)
                    continue;
                else
                {
                    cout<<endl;
                    for(k=0;k<15;k++)
                    {
                        cout<<allLaptops[lap[x]-1][k];
                    }
                    cout<<"\t \t \t \t"<<priceLaptops[lap[x]-1];
                }
            }
            cout<<endl;
        }
        if(checkMob==1) //MOBILE CHECK
        {
            cout<<"\n Mobile"<<endl;
            for(y=0;mob[y]<=5;y++)
            {
                if((mob[y]-1)<0)
                    continue;
                else
                {
                    cout<<endl;
                    for(k=0;k<15;k++)
                    {
                        cout<<allMobiles[mob[y]-1][k];
                    }
                    cout<<"\t \t \t \t"<<priceMobiles[mob[y]-1];
                }
            }
            cout<<endl;
        }
        if(checkTV==1) //TV CHECK
        {
            cout<<"\n Television"<<endl;
            for(z=0;tv[z]<=5;z++)
            {
                if((tv[z]-1)<0)
                    continue;
                else
                {
                    cout<<endl;
                    for(k=0;k<15;k++)
                    {
                        cout<<allTVs[tv[z]-1][k];
                    }
                    cout<<"\t \t \t \t"<<priceTVs[tv[z]-1];
                }
            }
            cout<<endl;
        }
        cout<<"\n \t \t \t \t Total bill== Rs "<<totalPrice;
        assignID();
        setDefault();//BY SETTING BACK ALL VALUES, PROGRAM CAN BE ALLOWED TO RUN AGAIN WITHOUT SHOWING PARTS OF PREVIOUS OUTPUTS
    }
}control;


There is more to the code but my question is regarding printBill(). After the TV check their is a for loop. Before i had a problem that the program hanged during that part when the for loop was
 
for(z=0;z<=sizeof(tv)/2;z++)


it was semantically the same for laptops and mobiles but they worked fine
So whats wrong with this loop
Last edited on
(z=0;tv[z]<=5;z++) should be (z = 0; z <= 5; z++)
If there wouldn't any entries with value larger that 5 in array, you will get infinite loop and duffer overdlow and crash.
but it works perfectly fine
its not supposed to take any value greater than 5, that is made sure by the enterItems()
its when i used
for(z=0;z<=sizeof(tv)/2;z++)
it crashed just for the tv case. But if i only entered laptops and mobiles it worked perfectly fine even though they have the same semantical condition.
by the way it prints wierd characters as well (like a smiley face xD)
for(z=0;z<=sizeof(tv)/2;z++)
will crash because most likely sizeof(int) == 4, i.e. out of bounds

write it like so:
for(z=0;z<=sizeof(tv)/sizeof(*tv);z++)


All the loop like
for(z=0;tv[z]<=5;z++)
have the same problem: if you enter a valid product id at position 0, what is the value at position 1?
im sorry im kind of new in c++. what did you mean by position 0 and 1?
but it works perfectly fine
its not supposed to take any value greater than 5, that is made sure by the enterItems()

If tv[z] never takes any number greater than 5, then your loop will never exit.

What will actually happen is that, once z reaches 10, it starts reading values from memory beyond the end of the array. In other words, it's reading values that are effectively random - you can't be sure what the compiler has decided to use that memory for. Eventually, one of these values will be higher than 5 (maybe even the first of them), and the loop will exit. But it's unpredictable, and presumably, it's not the way you actually intended it to behave.

What are you actually trying to achieve with that loop?
im sorry im kind of new in c++. what did you mean by position 0 and 1?
postion 0: tv[0] etc.

You set all fields of tv, lap, etc. to -1 which is <= 5, hence
for(z=0;tv[z]<=5;z++)
will always lead to z (and the other) is going out of bounds

you should write this
for(z=0;(tv[z]>0) && (tv[z]<=5);z++)
and take care the there's always one field left (or additionally limit z to sizeof(tv)/sizeof(*tv))
ok understood thanks coder777

@MikeyBoy i was using the array tv[] to store the values that the user entered in the TVCHECK part in the function enterItem() so that they can be used to later to print out the items that the user bought. After the bill is printed, setDefault() changes all the variables back to 0 so that the variables can be used in the next instance (its a menu program).
OK... but I don't understand why you're comparing your loop counter variable to the values stored in tv[] to check whether to continue looping or not. How many times did you intend it to loop? When did you intend it to finish looping?
i inteded it to finish looping after all the values that the user entered. For example: if the user enters the following values
1
2
3
4
5
0 //0 does not get sent to tv[]
so therefore tv[10]={1,2,3,4,5,0,0,0,0,0} //Maybe i got the zero's wrong but you get the idea

So after 5 i want the loop to stop so that it doesnt perform it again unnecessarily .
But what you're actually doing is looping until you happen to find a number in memory that's greater than 5. That can only happen once you go past the end of your array (because you initialise all members of the array to be -1, and you only allow numbers from 1 - 5 to be entered), in which case you're in the territory of stuff you can't predict. What if the first number after the end of the array happens to be a 1? What if it happens to be -10,000?

Also, why is your option class inheriting from laptops, mobiles and television? That doesn't seem to be intuitive to me. Inheritence models an "is-a" relationship. Is it true that an "option" is a television, and is a laptops, and is a mobiles, all at the same time? Multiple inheritence is rather a, well, brave thing to attempt, unless you're absolutely sure you know what you're doing.
Last edited on
@MikeyBoy yes now i know my condition is wrong.

Yes you are right option is not always a laptop, mobile and tv but it some times is. The program allows you to buy any of the items(in any category i.e. laptop, mobile and tv) for a single bill. Im not sure if that helps you to understand why i used multiple inheritance.

Also, could you explain why multiple inheritance is a brave thing to do?
Topic archived. No new replies allowed.