Thread: serious logical error!

  1. #1
    Registered User
    Join Date
    Apr 2013
    Posts
    103

    serious logical error!

    my project is on an online shop in which you can manage a shop(adding ,deleting ,clearing and modifying items) also you can switch to different shops(all shops are same)
    Code:
    #include<iostream>
    #include<fstream>
    #include<conio.h>
    #include<stdlib.h>
    #include<string.h>
    //#include<>
    using namespace std;
    class item{
            char name[20];
            int price;
        public:
            char* gname()
            {
                return name;
            }
            int gprice()
            {
                return price;
            }
            void input()
            {
                cout<<"enter new item name : ";
                cin>>name;
                cout<<"enter its price     : ";
                cin>>price;
            }
            void disp()
            {
                cout<<"\nitem name : "<<name;
                cout<<"\nprice     : "<<price<<"\n";
            }
            void modify()
            {
                cout<<"enter new price((-1) to keep old price) : ";
                cin>>tprice;
                if(tprice!=-1)
                prive=tprice;
                cout<<"enter new name(. to keep old name)      : ";
                cin>>tname;
                if(strcmp(tname,'.')!=0)
                strcpy(name,tname);
            }
    };
    class newuser{
            char name[20],pass[30];
            int shopno,subsc;
        public:
            newuser()
            {
                strcpy(name,"\0");
                strcpy(pass,"\0");
                shopno=0;
                subsc=0;
            }
            void getdata(char user[],char pas[],int shopn,int track)
            {
                
                strcpy(name,user);
                strcpy(pass,pas);
                shopno=shopn;
                subsc=track;
            }
            int getshopno()
            {
                return shopno;
            }
            int match(char user[],char pas[])
            {
                if(strcmp(user,name)==0&&strcmp(pas,pass)==0)
                {
                    return 1;
                }
                else
                {
                    return 0;
                }
            }
            void copy(newuser *cur)
            {
                cur->shopno=shopno;
                strcpy(cur->name,name);
                strcpy(cur->pass,pass);
                cur->subsc=subsc;
            }
            char* getname()
            {
                return name;
            }
            int getsub()
            {
                return subsc;
            }
            void shop(int,char);
    };
    void newuser :: shop(int track,char mode)
    {
        clrscr();
        char namef[]={track+96,'.','t','x','t','\0'},name[20];
        int i;
        item a;
        if(mode=='a')
        {
            a.input();
            ofstream inp(namef,ios::app);
            inp.write((char*)&a,sizeof(a));
            cout<<"new item added!";
            inp.close();
        }
        else if(mode=='s')
        {
            //display lines start---------------------------------
                    ifstream outp(namef);
            cout<<"you have the following items :-\n";
                    while(!outp.eof())
            {
                outp.read((char*)&a,sizeof(a));
                a.disp();
            }
            outp.close();
                    //end-------------------------------------------------
        }
        else if(mode=='d')
        {
            //display lines start---------------------------------
            ifstream outp(namef);
            cout<<"you have the following items :-\n";
            while(!outp.eof())
            {
                outp.read((char*)&a,sizeof(a));
                a.disp();
            }
            //end-------------------------------------------------
            outp.close();
            cout<<"enter the item name to be deleted : ";
            cin>>name;
            cout<<"item deleted!";
            cout<<"\nlist after deletion :-";
            ifstream outf(namef);
            ofstream inp("temp.txt");
            while(!outf.eof())
            {
                outf.read((char*)&a,sizeof(a));
                if(strcmp(a.gname(),name)==0)
                {
                    continue;
                }
                inp.write((char*)&a,sizeof(a));
                a.disp();
            }//end of while
            inp.close();
            outf.close();
            remove(namef);
            rename("temp.txt",namef);
        }
        else if(mode=='m')
        {
            item newitem;
            //display lines start---------------------------------
            ifstream outp(namef);
            cout<<"you have the following items :-\n";
            while(!outp.eof())
            {
                outp.read((char*)&a,sizeof(a));
                a.disp();
            }
            outp.close();
            //end-------------------------------------------------
            cout<<"enter the item name to be modified : ";
            cin>>name;
            newitem.modify();
            cout<<"item modified!";
            ifstream outf(namef);
            ofstream inp("temp.txt");
            while(!outf.eof())
            {
                outf.read((char*)&a,sizeof(a));
                if(strcmp(a.gname(),name)==0)
                {
                    inp.write((char*)&newitem,sizeof(newitem));
                }
                else
                {
                    inp.write((char*)&a,sizeof(a));
                }
            }//end of while
            inp.close();
            outf.close();
            remove(namef);
            rename("temp.txt",namef);
            cout<<"list after modification :-\n";
            ifstream dispf(namef);
            while(!dispf.eof())
            {
                dispf.read((char*)&a,sizeof(a));
                a.disp();
            }//end of while
            dispf.close();
        }
        else if(mode=='c')
        {
            remove(namef);
            ofstream newf(namef);
            newf.close();
            cout<<"shop cleared!";
        }//end if if-else...
        getch();
    }//end of function void shop();
    int run()
    {
        newuser s[25],cur;
        char user[20],pass[30];
        int choice,i,j,track=0,subsc=0,shopno;
        for(int z=0;z!=1;)
        {
            clrscr();
            cout<<"\t\t\twelcome to the online shop\n\n\n";
            if(cur.getshopno()==0)
            {
                cout<<"you are not registered or logged in\n";
                cout<<"1)log in\n2)register\n3)exit\n";
                cin>>choice;
                clrscr();
                switch(choice)
                {
                    case 1:{
                        int check=0;
                        cout<<"enter user name : ";
                        cin>>user;
                        cout<<"enter password  : ";
                        cin>>pass;
                        for(i=0;s[i].getshopno()!=0;i++)
                        {
                            if(s[i].match(user,pass))
                            {
                                s[i].copy(&cur);
                                subsc=cur.getsub();
                                check++;
                                break;
                            }
                        }
                        if(check==0)
                        {
                            cout<<"username or password is incorrect\n";
                            cout<<"getting you back to home screen...";
                            getch();
                        }
                        break;
                    }//end of case-1
                    case 2:{
                        if(track==25)
                        {
                                cout<<"this system can bear no more users\n";
                            cout<<"getting you back to the main screen...";
                            getch();
                            break;
                        }
                        cout<<"enter your user name : ";
                        cin>>user;
                        cout<<"enter your password  : ";
                        cin>>pass;
                        cout<<"enter your shopno    : ";
                        cin>>shopno;
                           s[track].getdata(user,pass,shopno,track);
                        s[track].copy(&cur);
                        track++;
                        subsc=track;
                        clrscr();
                        break;
                    }//end of case-2
                    case 3:{
                        cout<<"exitting....";
                        getch();
                        return track;
                        break;
                    }//end of case-3
                }//end of switch
            }//end of if(cur.shopno()==0)
            else
            {
                cout<<cur.getname()<<"\n\n";
                cout<<"what would you like to do ?\n";
                cout<<"\n1)check your shop";
                cout<<"\n2)add one item";
                cout<<"\n3)delete one item";
                cout<<"\n4)modify one item";
                cout<<"\n5)clear your shop";
                cout<<"\n6)log out\n";
                cin>>choice;
                switch(choice)
                {
                    case 1:{
                        cur.shop(subsc,'s');
                        break;
                    }
                    case 2:{
                        cur.shop(subsc,'a');
                        break;
                    }
                    case 3:{
                        cur.shop(subsc,'d');
                        break;
                    }
                    case 4:{
                        cur.shop(subsc,'m');
                        break;
                    }
                    case 5:{
                        cout<<"are you sure want to clear your shop ?(y/n)";
                        if(getch()=='y')
                        {
                            cur.shop(subsc,'c');
                        }
                        break;
                    }
                    case 6:{
                        cur.getdata(user,pass,0,0);
                        break;
                    }
                }
            }
        }//end of main loop
    }//end of function int run()
    char* getnamef(int pos)
    {
        char arr[]={pos+96,'.','t','x','t','\0'};
        return arr;
    }
    int main()
    {
        clrscr();
        int stat=run(),i=1;
        /*removing the files before exitting*/
        char arr[]={i+96,'.','t','x','t','\0'};
        for(;i<=stat;i++)
        {
            remove(arr);
            arr[0]++;
        }
        return stat;
    }
    the logical problem is serious because i am not able to find the error!!!
    there is some problem with display lines in void shop()-->if(mode=='s') and if(mode=='d')
    i add only one item but in output screen i see two items(sometimes >2)
    *note*
    to run the above display lines in the output screen....first register--->add atleast one item--->see your shop

  2. #2
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    You are using eof to control a loop, which is a bad idea. There's an FAQ about this if you go to the FAQs.

    Generally speaking, instead of using eof to control a loop, you should check whether the read succeeded as your loop control.

  3. #3
    Registered User
    Join Date
    Apr 2013
    Posts
    103
    @tabstop....how should i check that ?

  4. #4
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Put the read in the while (as opposed to the loop).

  5. #5
    Registered User
    Join Date
    Apr 2013
    Posts
    103
    @tabstop...i found another way.
    i used a static variable to count number of items(initilized with 0...++ if i add.....-- if i delete) in function void shop in class item and used a for loop instead and that removed the error!!! thanks tabstop you told me about that eof() function.

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by Mukul Kumar
    i used a static variable to count number of items(initilized with 0...++ if i add.....-- if i delete) in function void shop in class item and used a for loop instead and that removed the error!
    That is generally a bad idea because it means that your member function is very tightly coupled to the particular piece of code in which you're calling it right now. You cannot reuse the function.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Registered User
    Join Date
    Apr 2013
    Posts
    103
    .........you are right i will find a way to solve this.

  8. #8
    Registered User
    Join Date
    Apr 2013
    Posts
    103
    okay how is this that i add last object in file with price -ve(obviously price in real life is never -ve) and as my loop detects price -ve i'll add a break statement there.

  9. #9
    Registered User
    Join Date
    Apr 2013
    Posts
    103
    yessss... i got it ...... i calculated the file size and then divide that with the object size....then i got the number of objects in that file(this thing worked successfully).

  10. #10
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by Mukul Kumar View Post
    yessss... i got it ...... i calculated the file size and then divide that with the object size....then i got the number of objects in that file(this thing worked successfully).
    So you end up reading the file twice? Why not just put the read in the while statement instead of inside the loop proper?

  11. #11
    Registered User
    Join Date
    Apr 2013
    Posts
    103
    noooo.... not reading a file twice....i opened file then seekp(0,ios::end) then fetched its pointer position....than tellp()/sizeof(item).....then seekp(0) thats how i got the file size(in terms of object)

  12. #12
    Registered User hk_mp5kpdw's Avatar
    Join Date
    Jan 2002
    Location
    Northern Virginia/Washington DC Metropolitan Area
    Posts
    3,817
    Code:
    class item{
        char name[20];
        int price;
    public:
        char* gname()
        {
            return name;
        }
        int gprice()
        {
            return price;
        }
       ...
    };
    
    class newuser{
        char name[20],pass[30];
        int shopno,subsc;
    public:
        ...
        int getshopno()
        {
            return shopno;
        }
        ....
        char* getname()
        {
            return name;
        }
        ...
    };
    Your are exposing a raw pointer in your implementation that could be used to alter the data in the two class' name members. This is dangerous. Functions which do not alter the state of the class should be declared const, also, the functions returning the pointers should do so as const pointers to ensure that the pointers cannot be used to change the data in the class. But really, you should be using std::string containers instead of char arrays/pointers.


    Code:
    class item{
        char name[20];
        int price;
    public:
        const char* gname() const
        {
            return name;
        }
        int gprice() const
        {
            return price;
        }
       ...
    };
    
    class newuser{
        char name[20],pass[30];
        int shopno,subsc;
    public:
        ...
        int getshopno() const
        {
            return shopno;
        }
        ....
        const char* getname() const
        {
            return name;
        }
        ...
    };
    There are other functions which can be declared const that you should look at since they do not change the object they are called on but I've just looked predominately at the ones above which return the pointers.
    "Owners of dogs will have noticed that, if you provide them with food and water and shelter and affection, they will think you are god. Whereas owners of cats are compelled to realize that, if you provide them with food and water and shelter and affection, they draw the conclusion that they are gods."
    -Christopher Hitchens

  13. #13
    Registered User
    Join Date
    Apr 2013
    Posts
    103
    sorry....i know that but in Turbo C++ this is not available....nothing is available....also i use modern editors and compilers but i have got a project from school and they have only Turbo C++....one day i mentioned many of its errors and mistakes and introduced new things(yeah that i use) but my teacher refused anyway.... thanks for putting light on this thing i will keep in mind.
    **also...the kind of education we get in computers is like feeding on garbage(they don't even know that system("cls") is so dangerous).
    Last edited by Mukul Kumar; 11-18-2013 at 07:34 PM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Logical error. e
    By ccl06905 in forum C Programming
    Replies: 5
    Last Post: 12-01-2011, 05:25 AM
  2. Logical Error
    By Luigi1 in forum C Programming
    Replies: 1
    Last Post: 02-27-2011, 02:41 PM
  3. Logical error with array
    By swgh in forum C++ Programming
    Replies: 6
    Last Post: 03-30-2009, 12:21 PM
  4. weirdest logical error i've ever run into
    By Leeman_s in forum C++ Programming
    Replies: 6
    Last Post: 05-26-2003, 10:50 AM
  5. Logical Error
    By gardenair in forum C Programming
    Replies: 2
    Last Post: 04-06-2003, 04:18 PM

Tags for this Thread