A vector in a struct - best approach? C++

Question

I am trying to include a vector in my struct.

Here is my struct:

struct Region 
{
    bool hasPoly;
    long size1; 
    long size2;
    long size3;
    long size4;
    long size5;
    long size6;
    //Mesh* meshRef; // the mesh with the polygons for this region
    long meshRef;
    std::vector<int> PVS;
} typedef Region;

Is the vector in this declaration valid or would it make more sense to do a pointer to a vector. In the case of a pointer to a vector, do I need to allocate a new vector. How would I accomplish this?

Thanks!

Edit: The problem is that it ends up causing an error that points to xmemory.h, a file included with the MSVC++ platform.

    void construct(pointer _Ptr, _Ty&& _Val)
        {   // construct object at _Ptr with value _Val
        ::new ((void _FARQ *)_Ptr) _Ty(_STD forward<_Ty>(_Val)); // this is the line
         }

Interestingly, it does not happen if I allocate it outside of the struct and simply in the function I use. Any ideas?

Bojorquez

This is perfectly valid. If you want to use a pointer, yes you must allocate the vector, in the constructor probably. – Junkie

I don't see the need for typedef there since you are using C++. – Spondaic

@Satchmo: I don't get that error in MSVC++ Express 10, with the code exactly as you defined it... – Consummation

It's a runtime error: access violation writing location. Very bizarre because it only happens with the vector when it's in the struct. If I declare a vector in the actual function, it works great. – Bojorquez

@Beta: If he gets it to compile right, then allocating in the constructor is semantically identical to a non-pointer member (except for the heap allocation part). And he'll still have to implement the "big three", proper RAII pattern, etc. – Consummation

@Merlyn Morgan-Graham My function is causing the error. This is the function. pastebin.com/Lxe2FQui Note: It only crashes with a vector when it's declared in the struct. If I use the function as it is, it works great. – Bojorquez

@Merlyn Morgan-Graham Would you suggest I turn my region into a class instead? Might make more sense. – Bojorquez

1) Your code refers to data and not to Region (is data a global variable of type Region?) 2) the only difference between a class and a struct is that the members are default-public in a struct and default-private in a class (and by tradition, people tend to use classes if they want to have member functions). – Junkie

@Satchmo: It depends on the usage. If you don't plan to put methods on it, then no, don't turn it into a class, even if you have a constructor. – Consummation

Wait... The code you showed us uses a local vector, but the error occurs only when you try to use a vector that belongs to a struct? There are a few things you could be doing wrong, and it's hard to tell what's happening if we can't see the actual code. – Junkie

@Satchmo - Where are you storing the result of the call? That might be an invalid address. – Poco

The code you show us is fine. The reason you have a crash only with the vector may be dangling references or pointers to your struct hat has gone out of scope. With PODs that may not be catastrophic (but just another memory writing bug ;-) ). A vector has dynamic memory which is deallocated upon destruction though. Accessing that usually is a crash. – Coccidiosis

Answer 1

You can write it like this without the typedef:

struct Region 
{
    bool hasPoly;
    long size1; 
    long size2;
    long size3;
    long size4;
    long size5;
    long size6;
    long meshRef;
    std::vector<int> PVS;
}; // no typedef required

To answer your questions:

Is the vector in this declaration valid

Yes, it is.

or would it make more sense to do a pointer to a vector.

No, probably not. If you did then you would have to implement copy constructor, assignment operator and destructor for the copy behavior. You would end up with the same but it would be extra work and potentially introduce bugs.

In the case of a pointer to a vector, do I need to allocate a new vector. How would I accomplish this?

You would need to implement the copy constructor, the copy assignment operator and the destructor:

// Copy constructor
Region(const Region & rhs) :
    hasPoly(rhs.hasPoly),
    // ... copy other members just like hasPoly above, except for PVS below:
    PVS(new std::vector<int>(*rhs.PVS))
{
}

// Copy assignment operator
Region & operator=(const Region & rhs)
{
    if (this != &rhs)
    {
         hasPoly = rhs.hasPoly;
         // ... copy all fields like hasPoly above, except for PVS below:

         delete PVS;
        PVS = new std::vector<int>(*rhs.PVS);
    }
    return *this;
}

// Destructor
Region::~Region()
{
    delete PVS;
}

Bottom line: your code is fine. You don't need to change it.

EDIT: Fix assignment operator: check for comparison against this and return *this.

Saker

+1. Big three usually calls for implementing copy-and-swap, too: #3280043 – Consummation

Isn't the typedef syntax wrong? I'd write typedef struct {} name; – Coccidiosis

Well, the OP doesn't have to change the code they showed us. – Coccidiosis

Answer 2

It makes complete sense to do that and you don't need new in any respect, unless you actually want to alias a separate vector. In addition, you don't need any typedef stuff going on here.

Falbala

Answer 3

It depends on how you use it.

If you want to copy the vector and data when copying the Region struct, then leave it as a non-pointer.

If you don't want it copied over, then you will want some sort of pointer to a vector.

If you use a pointer to a vector, you should be very careful about allocation/deallocation exception safety. If you can't scope your allocation in an exception safe way, then you'll leave a potential for memory leaks.

A couple options are:

• Make sure that the code that allocates the vector (and uses the Region) also deallocates the vector, and is itself exception safe. This would require the Region to only exist inside that code's scope.
  You could do this by simply allocating the vector on the stack, and pass that to the pointer in the Region. Then make sure you never return a Region object above that stack frame.
• You could also use some sort of smart pointer -> vector in your Region.

Consummation

See the edit for the problem that is happening. Sorry. new to this site :) – Bojorquez

Answer 4

The vector is fine. Be aware that if you copy this struct, then the vector will be copied with it. So in code with particular performance constraints, treat this struct the same way that you'd treat any other expensive-to-copy type.

In production code, some people would prefer you to use the class keyword rather than the struct keyword to define this class, since the vector member makes it non-POD. If you're the author of your own style guide there's nothing to worry about.

The typedef is wrong, though, just write struct Region { stuff };

Manzanilla

Answer 5

typedef struct Region {
  bool hasPoly;
  long size1; 
  long size2;
  long size3;
  long size4;
  long size5;
  long size6;
  long meshRef;
  std::vector<int> PVS;
}R_t;

Is fine.you can later create "Region" type objects by doing:

R_t a, b;

Ladonnalady