Program I made!

This is a discussion on Program I made! within the C++ Programming forums, part of the General Programming Boards category; it is still the first version, for those who want to help me learn c++, u can check it out ...

  1. #1
    Registered User
    Join Date
    Nov 2006
    Posts
    86

    Program I made!

    it is still the first version, for those who want to help me learn c++, u can check it out and maby give me some tips or hints and Ideas for an upgrade

    tnx!

    this program is sort of an database, which stores records ( classes ) on a file and , reads writes or deletes them

    ( i wanted too upload a rar pack containing it all but thats not allowed )

    -> in the future i want it too be some sort of client, where u can login using your password etc, and if you are an "admin" u have MORE powers then an normal member etc
    Last edited by epidemic; 04-06-2007 at 03:58 PM. Reason: program is messy... needs work

  2. #2
    For Narnia! Sentral's Avatar
    Join Date
    May 2005
    Location
    Narnia
    Posts
    719
    You use goto a lot. This produces very hard to read code, and is a bad habit.
    Videogame Memories!
    A site dedicated to keeping videogame memories alive!

    http://www.videogamememories.com/
    Share your experiences with us now!

    "We will game forever!"

  3. #3
    Registered User
    Join Date
    Nov 2004
    Location
    Pennsylvania
    Posts
    434
    Goto = Bad, haha

    In C++ we try to make Object-Oriented Programs, goto is used in linear programs.

    If you wanna try something new try writing the same program without using goto

    Have fun!
    "Anyone can aspire to greatness if they try hard enough."
    - Me

  4. #4
    Registered User
    Join Date
    Nov 2006
    Posts
    86
    LOL? i used goto 2 times as in GOTO MAINDRIVER and GOTO skip.

    whats so bad about goto? lol why should i make the same function calls over and over on the end of an task u picked if i can just use one goto to the beginning of the program where the function is just called once?.

    if i replaced all Goto MAINDRIVER ( which are 4 calles ) with this :

    PrintMenu ();
    choise = IntegerInputHandler ( lengths,index );


    , it will just make my application size bigger for no need. as for goto skip, i do not know how i could deal with that in an other way:

    Code:
    void ReadSingleRecord ( unsigned int id, int times )
    {
      unsigned int LOCATION = 0;
      member empty (" "," "," ", 0 , 0 );
      std::ifstream ReadOut ("C:\\Datacab.dat", ios::in | ios::binary );
      if (!ReadOut)
      {
        CheckFile ();
      }
      else
      {
        ReadOut.seekg ( 0 , ios::beg );
        while (!ReadOut.eof() )
        {
          ReadOut.read ( ( reinterpret_cast < char * > (&empty) ),sizeof (member) );
          LOCATION++;
          if (empty.getaccnum ()  == id )
          {
            LOCATION--;
            break;
          }
        }
        ReadOut.seekg ( ( LOCATION* sizeof (member) ), ios::beg );
        if ( empty.getaccnum ()  == 0 )
        {
          std::cout <<"Record does not exist \n\n" << std::endl;
          goto skip;
        }
        for (int x = 0; x < times; x++)
        {
          ReadOut.read ( ( reinterpret_cast < char * > (&empty) ), sizeof (member) );
    
          std::cout <<"Record Number " << LOCATION << std::endl;
          std::cout <<"_______________\n"
                    <<"Username: " << empty.getusername () <<"\n"
                    <<"Email: " << empty.getemail () <<"\n"
                    <<"Password: " << empty.getpassword () <<"\n"
                    <<"Age: " << empty.getage () <<"\n"
                    <<"Accountnumber: " << empty.getaccnum () <<"\n\n"<< std::endl;
        }
        skip:
    
      }
    }
    ok lol i could implement that if ( empty.getaccnum () == 0 ) into my loop, but that just waisting proccessing time.
    (in my eyes)
    Last edited by epidemic; 04-06-2007 at 01:27 PM.

  5. #5
    Registered User
    Join Date
    Mar 2007
    Posts
    416
    I wrote different functions for my game depending on if people wanted to play again, then had the function call itself. It's just something different from goto.

    Code:
    #include <allegro.h>
    
    void EasyMode(){
        while(stuff){
            //stuff, whatever can go here
    
            textout(screen, font, "play again?", 0, 0, makecol(255,0,0),makecol(0,0,0));
            if(key[KEY_Y]) EasyMode();
            if(key[KEY_N]) break;
        }
    }

  6. #6
    Registered User
    Join Date
    Apr 2007
    Location
    Sweden
    Posts
    41
    Quote Originally Posted by epidemic View Post
    , it will just make my application size bigger for no need. as for goto skip, i do not know how i could deal with that in an other way:

    Code:
     // snip
        if ( empty.getaccnum ()  == 0 )
        {
          std::cout <<"Record does not exist \n\n" << std::endl;
          goto skip;
        }
        for (int x = 0; x < times; x++)
        {
          ReadOut.read ( ( reinterpret_cast < char * > (&empty) ), sizeof (member) );
    
          std::cout <<"Record Number " << LOCATION << std::endl;
          std::cout <<"_______________\n"
                    <<"Username: " << empty.getusername () <<"\n"
                    <<"Email: " << empty.getemail () <<"\n"
                    <<"Password: " << empty.getpassword () <<"\n"
                    <<"Age: " << empty.getage () <<"\n"
                    <<"Accountnumber: " << empty.getaccnum () <<"\n\n"<< std::endl;
        }
        skip:
    
      }
    ok lol i could implement that if ( empty.getaccnum () == 0 ) into my loop, but that just waisting proccessing time.
    (in my eyes)
    Like this?
    Code:
        if ( empty.getaccnum ()  == 0 )
        {
          std::cout <<"Record does not exist \n\n" << std::endl;
        }
        else
        {
            for (int x = 0; x < times; x++)
            {
              ReadOut.read ( ( reinterpret_cast < char * > (&empty) ), sizeof (member) );
    
              std::cout <<"Record Number " << LOCATION << std::endl;
              std::cout <<"_______________\n"
                        <<"Username: " << empty.getusername () <<"\n"
                        <<"Email: " << empty.getemail () <<"\n"
                        <<"Password: " << empty.getpassword () <<"\n"
                        <<"Age: " << empty.getage () <<"\n"
                        <<"Accountnumber: " << empty.getaccnum () <<"\n\n"<< std::endl;
            }
        }
    A lot easier to follow if you ask me.

  7. #7
    For Narnia! Sentral's Avatar
    Join Date
    May 2005
    Location
    Narnia
    Posts
    719
    You seem to have the wrong attitude epidemic. It's not about the size of the program, it's about the program's readability and flow. Think about it. Keep in mind, others have to read it.
    Videogame Memories!
    A site dedicated to keeping videogame memories alive!

    http://www.videogamememories.com/
    Share your experiences with us now!

    "We will game forever!"

  8. #8
    Registered User
    Join Date
    Nov 2006
    Posts
    86
    Quote Originally Posted by Sentral View Post
    You seem to have the wrong attitude epidemic. It's not about the size of the program, it's about the program's readability and flow. Think about it. Keep in mind, others have to read it.
    It is readable. very readable if u ask me. once u know that goto MAINDRIVER just asks for input again for the main menu... whats so hard to read it? it just "restarts" the program..

    oysterman, k lol ty

  9. #9
    The larch
    Join Date
    May 2006
    Posts
    3,573
    I don't get the ReadSingleRecord function. First you search for a record that has a matching id. If you find it then - instead of printing it right away and returning - you go through some sphagetti to print a whole bunch of records?

    The skip is not needed, because you could have done whatever you are trying to skip later, already (if (empty.getaccnum () == id) -> do it here).

    If the indentations go too deep, break it up into smaller functions.

    (If you say it is very readable, why do you want comments on it? The only function I have seen is not readable at all: starting from the name of the function which seems off.)

    Edit: Took a look at the rest. Not too bad, except for a few magic numbers. In main you probably don't seem to need the goto either. Wouldn't you get to the exact same place (top of while) if you just breaked each case of the switch like people are used to?
    Last edited by anon; 04-06-2007 at 02:46 PM.

  10. #10
    The larch
    Join Date
    May 2006
    Posts
    3,573
    Quote Originally Posted by scwizzo View Post
    I wrote different functions for my game depending on if people wanted to play again, then had the function call itself. It's just something different from goto.

    Code:
    #include <allegro.h>
    
    void EasyMode(){
        while(stuff){
            //stuff, whatever can go here
    
            textout(screen, font, "play again?", 0, 0, makecol(255,0,0),makecol(0,0,0));
            if(key[KEY_Y]) EasyMode();
            if(key[KEY_N]) break;
        }
    }

    This is not a really good idea. You do realize that each time you go deeper into recursion, the stack space is getting smaller and smaller.

    Code:
    void EasyMode(){
        while(true){
            PlayOneGame();
    
            textout(screen, font, "play again?", 0, 0, makecol(255,0,0),makecol(0,0,0));
            if(!key[KEY_Y]) break;
        }
    }

  11. #11
    Registered User
    Join Date
    Nov 2006
    Posts
    86
    Quote Originally Posted by anon View Post
    I don't get the ReadSingleRecord function. First you search for a record that has a matching id. If you find it then - instead of printing it right away and returning - you go through some sphagetti to print a whole bunch of records?

    The skip is not needed, because you could have done whatever you are trying to skip later, already (if (empty.getaccnum () == id) -> do it here).

    If the indentations go too deep, break it up into smaller functions.

    (If you say it is very readable, why do you want comments on it? The only function I have seen is not readable at all: starting from the name of the function which seems off.)

    u can also read out blocks of 10 records ( times ) with the function, u give in the first record number, it finds the record, and then reads 10 records, because I wanted it to be easy to update, I gave a variable int times too the function so in the future, u can choose howmany records u can read out at once.


    ill change the function's name...


    Code:
    void ReadRecord ( unsigned int id, int times ) 
    {
      unsigned int LOCATION = 0;  // (location in file)/ sizeof(member)
      member empty (" "," "," ", 0 , 0 ); // blank member
      std::ifstream ReadOut ("C:\\Datacab.dat", ios::in | ios::binary );
      if (!ReadOut)
      {
        CheckFile ();
      }
      else
      {
        ReadOut.seekg ( 0 , ios::beg ); // position file pointer into the begining
        while (!ReadOut.eof() ) // while end of file is not reached
        {
          ReadOut.read ( ( reinterpret_cast < char * > (&empty) ),sizeof (member) );
          LOCATION++; // add one too location ( 1 * sizeof (member) )
          if (empty.getaccnum ()  == id )
          {
            LOCATION--;
            break;
          }
        }
        ReadOut.seekg ( ( LOCATION* sizeof (member) ), ios::beg ); // find actual location 
        for (int x = 0; x < times; x++)
        {
          if ( empty.getaccnum ()  == 0 ) // if accnum equals 0, record does not exist
          {
            std::cout <<"Record Number " << LOCATION << " does not exist" << std::endl;
          }
    
          ReadOut.read ( ( reinterpret_cast < char * > (&empty) ), sizeof (member) );
      
          std::cout <<"Record Number " << LOCATION << std::endl;
          std::cout <<"_______________\n"
                    <<"Username: " << empty.getusername () <<"\n"
                    <<"Email: " << empty.getemail () <<"\n"
                    <<"Password: " << empty.getpassword () <<"\n"
                    <<"Age: " << empty.getage () <<"\n"
                    <<"Accountnumber: " << empty.getaccnum () <<"\n\n"<< std::endl;
        }
        LOCATION += 1;  // location is not the same annymore (LOOP)
      }
    }
    Last edited by epidemic; 04-06-2007 at 02:56 PM. Reason: code:

  12. #12
    The larch
    Join Date
    May 2006
    Posts
    3,573
    ReadRecord still implies one record.

    I think the user of this function would be a lot happier, if the function just returned a member object with the matching ID. This way they can decide what to do with it: how they want to output it, may-be they want to edit it and write it back to the file etc.

    So I suggest something like that

    Code:
    member GetRecordById ( unsigned int id)
    {
      member temp; //provide a default constructor
    
      //ios::in is probably redundant for ifstream
      std::ifstream ReadOut ("C:\\Datacab.dat", ios::in | ios::binary );
      if (!ReadOut)
      {
        CheckFile ();
      }
      else
      {
        //it is not wise to control file input with eof()!
        while (ReadOut.read( reinterpret_cast<char*>(&temp), sizeof(member))
        {
          if (temp.getaccnum ()  == id )
          {
            return temp;
          }
        }
      }
      return member(); //return default constructed empty record
    }
    By the way, your code doesn't compile. You are trying to pass something like (var +1 ) to a function that expects int&. The reference means that the function may modify the original value, but alas - (var + 1) is a constant value. If you didn't mean to change the referenced value, don't pass it as a reference: references are more expensive for built-in data-types than copies.
    Last edited by anon; 04-06-2007 at 03:30 PM.

  13. #13
    Registered User
    Join Date
    Nov 2006
    Posts
    86
    It compiles with my compiler...
    but tnx for your post... gave me some good ideas

    Looking back, i am ashamed i posted this messy program ... it did work, and it did do what i wanted, but it still needs a lot of work. ( deleted added files )
    Last edited by epidemic; 04-06-2007 at 03:57 PM.

  14. #14
    Registered User manofsteel972's Avatar
    Join Date
    Mar 2004
    Posts
    317
    Don't be ashamed epidemic. Everyone starts somewhere and if you are just learning, well you were given some advice on how to improve. It is kind of like learning to draw. The first few are not going to be perfect, but you get to review and refine your methods. If you learn a way to improve your program, you can do it again taking advantage of the new methods.
    "Knowledge is proud that she knows so much; Wisdom is humble that she knows no more."
    -- Cowper

    Operating Systems=Slackware Linux 9.1,Windows 98/Xp
    Compilers=gcc 3.2.3, Visual C++ 6.0, DevC++(Mingw)

    You may teach a person from now until doom's day, but that person will only know what he learns himself.

    Now I know what doesn't work.

    A problem is understood by solving it, not by pondering it.

    For a bit of humor check out xkcd web comic http://xkcd.com/235/

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 1
    Last Post: 12-30-2007, 09:08 AM
  2. my server program auto shut down
    By hanhao in forum Networking/Device Communication
    Replies: 1
    Last Post: 03-13-2004, 09:49 PM
  3. insufficient memory for tsr
    By manmohan in forum C Programming
    Replies: 8
    Last Post: 01-02-2004, 08:48 AM
  4. Date program starts DOS's date
    By jrahhali in forum C++ Programming
    Replies: 1
    Last Post: 11-24-2003, 04:23 PM
  5. redirection program help needed??
    By Unregistered in forum Linux Programming
    Replies: 0
    Last Post: 04-17-2002, 05:50 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21