[C++] Class Vector

Discussion in 'Game Development (Technical)' started by Tanius, Jun 20, 2010.

  1. Tanius

    Tanius New Member

    Joined:
    May 7, 2010
    Messages:
    2
    Likes Received:
    0
    Hi everybody.

    I'm recently having problems with the Vector <> class.
    Precisely i want to make up a vector of class Material Instances.
    the declaration is in the class Geometry, like this

    Code:
     vector <Material> MaterialArray;
     long int NumMaterials;
     // Array dei materiali;
    I start filling the vector in the constructor of class Geometry like this.

    Code:
    for(int ind=0; ind<NumSets; ind++)
        {       
         //Prelevo il nodo
         aux=geometria.getChildNode("triangles",ind);
         
         //Prelevo Nome Materiale e Aggiungo alla Lista
         Material m=Material();     
         m.set_name(aux.getAttribute("material"));     
         m.cpy=0;     
         printf("\n--- Aggiungo: %s",m.get_name());
         getchar();
    
         MaterialArray.push_back(m); 
         printf("\n--- Fatto");
    The push_back() method calls the copy constructor of Material, e.i.

    Code:
    Material::Material(const Material &m)
    {        
     name=NULL;
     path=NULL;                  
     id=m.id;
     cpy=m.cpy;
     colori=m.colori;
     a=m.a;  r=m.r;  g=m.g;  b=m.b;
     if(m.name!=NULL)
       {
        name=new char[strlen(m.name)];
        strcpy(name,m.name); 
       }                             
     if(m.path!=NULL)
       {   
        path=new char[strlen(m.path)]; 
        strcpy(path,m.path);       
        fi=JPGFile(m.fi);     
       }                             
    }
    the simple constructor of Material is:

    Code:
    Material::Material()
    {
     name=NULL;
     path=NULL;
    }
    At runtime, at the second iteration of the for in in the constructor of Geometry the app crashes... What am i missing?
     
  2. zoombapup

    Moderator Original Member

    Joined:
    Nov 25, 2004
    Messages:
    2,890
    Likes Received:
    0
    It looks like youre not actually constructing a new material properly.

    Either you need to store a Material * in the vector and remember to NEW the Material.

    Or

    You are calling push back, but are you sure the actual material is being constructed properly? Maybe its a scoping issue?

    The line Material m = Material(); looks very iffy. Are you trying to construct the material? In which case the constructor will be called (you dont need the =Material(); bit), if not, then you need a Material * in the vector.
     
    #2 zoombapup, Jun 20, 2010
    Last edited: Jun 20, 2010
  3. Bmc

    Bmc New Member

    Joined:
    Dec 12, 2004
    Messages:
    1,088
    Likes Received:
    2
    this line doesn't make sense

    Material m=Material();

    Should just be
    Material m;
     
  4. Tanius

    Tanius New Member

    Joined:
    May 7, 2010
    Messages:
    2
    Likes Received:
    0
    I simply want to make a dynamic array of class Material... How'd you code that? Thanks^^
     
  5. sindisil

    Original Member

    Joined:
    Feb 26, 2006
    Messages:
    115
    Likes Received:
    0
    and this
    are wrong.

    A null terminated string needs room for the null terminator! strcpy is overwriting the buffer you allocated by sizeof (char).
    Code:
    {
        int slen = strlen(m.path);
        path = new char[slen + 1];
        strncpy(path, m.path, slen);
    }
    
    is what you wanted to say, assuming you're using null terminated strings on purpose. There are certainly plenty of reasons why you might want to use them instead of <string>, but one downside is needing to always be cognizant of the terminator. Eventually it comes naturally, but it's one reason why <string> exists in C++.

    I'm not saying that there isn't anything else wrong there, but that immediately jumped out at me, since I spend most of my time in C, rather than C++.
     
    #5 sindisil, Jun 20, 2010
    Last edited: Jun 20, 2010
  6. chrisroyale

    Original Member

    Joined:
    Apr 19, 2005
    Messages:
    170
    Likes Received:
    0
    You should consider using a shared pointer such as boost::shared_ptr for. your material objects. That way you can pass references around instead of copies, and if the vector gets destroyed all other references remain valid. Code gets slightly more complex but life gets easier.

    Also, if material.cpy=0 i@ the default value you should put it in the class constructor initialization list. You should always be using constructor initialization lists when possible.
     
    #6 chrisroyale, Jun 26, 2010
    Last edited: Jun 26, 2010
  7. hippocoder

    Indie Author

    Joined:
    Mar 18, 2008
    Messages:
    591
    Likes Received:
    0
    Just go with readable code. Vectors are the spawn of satan and serve no purpose in making indie games imho. Linked lists, slower code, results = win.
     
  8. chrisroyale

    Original Member

    Joined:
    Apr 19, 2005
    Messages:
    170
    Likes Received:
    0
    Vectors have their place, when you need fast random access but not necessarily fast insert/delete. Just like linked lists have linear complexity on access but quick manipulation. Horses for courses, but all have their place.
     
  9. vjvj

    Indie Author

    Joined:
    Sep 25, 2004
    Messages:
    1,732
    Likes Received:
    0
    I banish you to the realm of cacheless CPUs for all eternity.

    Though in this case I agree <list> would be a better fit.
     
  10. chrisroyale

    Original Member

    Joined:
    Apr 19, 2005
    Messages:
    170
    Likes Received:
    0
    But since these are allocated in the geometry constructor isn't it more likely the contents of the collection won't change? If that's the case then a vector would be the correct choice, imho. ;)
     
  11. Pogacha

    Original Member

    Joined:
    Jan 21, 2005
    Messages:
    605
    Likes Received:
    0
    When pushing back into a vector, internally the vector template class might allocate a higher memory chunck copy all the materials using the copy constructor.
    If you know the number of materials ahead that wouldn't be a problem, using the function vector::reserve( int )

    with a linked list:

    typedef list<Material> MaterialsContainer;
    MaterialsContainer aMaterialsContainer;

    // the preferred option
    aMaterialsContainer.push_back( Materials( "bricks", "path/here/", Red ) );

    // the second preferred option
    aMaterialsContainer.push_back( Materials() );
    Material& m = aMaterialsContainer.back();
    m.Set_Path( "path/here" );
    m.Set_Name( "bricks" );

    As well it would be a millon times preferred to just use std::string for the names and paths.
     
  12. chrisroyale

    Original Member

    Joined:
    Apr 19, 2005
    Messages:
    170
    Likes Received:
    0
    @pogacha

    Vector::reserve may also reserve more space than you request. Parameter is a minimum value, and memory management is internal.
     
  13. Pogacha

    Original Member

    Joined:
    Jan 21, 2005
    Messages:
    605
    Likes Received:
    0
    Not sure why the clarification on this since std::vector will always allocate more space than requiered at its will and discretion, despite you use reserve, resize or push_back.
    That's why std::vector::reserve is for, to hint the std::vector allocator to avoid non desired penalties when sequential resizing commonly found while using push_back.
     
  14. JarkkoL

    JarkkoL New Member

    Joined:
    Feb 4, 2009
    Messages:
    363
    Likes Received:
    0
    Just store pointers to Material in the vector instead.
     

Share This Page

  • About Indie Gamer

    When the original Dexterity Forums closed in 2004, Indie Gamer was born and a diverse community has grown out of a passion for creating great games. Here you will find over 10 years of in-depth discussion on game design, the business of game development, and marketing/sales. Indie Gamer also provides a friendly place to meet up with other Developers, Artists, Composers and Writers.
  • Buy us a beer!

    Indie Gamer is delicately held together by a single poor bastard who thankfully gets help from various community volunteers. If you frequent this site or have found value in something you've learned here, help keep the site running by donating a few dollars (for beer of course)!

    Sure, I'll Buy You a Beer