Problem in for loop condition using sizeof()

[dedlier]

hey i made a program that calculates bills

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

[MiiNiPaa]

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

[dedlier]

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)

[coder777]

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?

[dedlier]

im sorry im kind of new in c++. what did you mean by position 0 and 1?

[MikeyBoy]

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?

[coder777]

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

[dedlier]

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

[MikeyBoy]

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?

[dedlier]

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 .

[MikeyBoy]

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.

[dedlier]

@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?