CODEWITHSUNDEEP
c++arraysstruct
Igor
Igor

Reputation: 558

C++ Set Array In Pointer to Struct

I have a struct in C++ as follows:

typedef struct DIFDict
{
    std::string name;
    std::string defs[];
    struct DIFColor *colors[];
    uint8_t defsize; 
} DIFDict;

The trouble is trying to initialize it. I have a function which returns a pointer to a struct with all the values initialized:

struct DIFDict *CreateDIFDict(std::string n, std::string *d, struct DIFColor **c, uint8_t de)
{
    int memsize = sizeof(n) + sizeof(d) + sizeof(c) + sizeof(de);
    struct DIFDict *i = static_cast<struct DIFDict*>(malloc(memsize));

    if(i != NULL)
    {
        i->name = n;
        i->defs = d;
        i->colors = c;
        i->defsize = de;
    }

    return i;

}

However, the compiler complains of type mismatches:

error: incompatible types in assignment of 'std::string {aka std::basic_string<char>}' to 'std::string [0] {aka std::basic_string<char> [0]}'
error: incompatible types in assignment of 'DIFColor**' to 'DIFColor* [0]'

Am I misunderstanding the relationship between pointers and arrays?

Upvotes: 0

Views: 238

Answers (3)

silent
silent

Reputation: 2904

typedef struct DIFDict
{
    std::string name;
    std::string *defs;
    struct DIFColor **colors;
    uint8_t defsize; 
} DIFDict;

struct DIFDict *CreateDIFDict(std::string n, std::string *d, struct DIFColor *c, uint8_t de)
{   
    DIFDict *i = 0;
    i = new DIFDict;

    if(i != 0)
    {
        i->name = n;
        i->defs = d;
        i->colors = &c;
        i->defsize = de;

        return i;
    }

    return 0;
}

Edit: The issue with the answer above is that the DIFColor parameter is a temporary, and pointer to the temporary will be invalid when the function exits. A more viable (not the best, but at least viable) solution that closely mimics the above would be as follows:

    struct DIFDict
    {
        std::string name;
        std::string *defs;
        DIFColor **colors;
        uint8_t defsize; 
    };

    DIFDict *CreateDIFDict(const std::string& n, std::string *d, 
                           DIFColor **c, uint8_t de) 
    {   
        DIFDict *i = new DIFDict;
        i->name = n;
        i->defs = d;
        i->colors = c;
        i->defsize = de;
        return i;
    }

Note that we do not now take the address of the temporary -- all we're passing the pointer value directly and having it assigned. Also, note that new does not return 0 on failure (unless the nothrow option is used), so no check is needed for new being 0.

However, this solution is still C-like and is error-prone, and there are much better methods, as outlined by the other answers given.

Upvotes: 2

M.M
M.M

Reputation: 141554

You seem to have some basic misunderstanding of arrays in C++. They have have a fixed size, known at compile-time. Zero-sized arrays are illegal. (Some compilers allow a zero-sized array when not invoked in standard mode, however they have unclear semantics, and certainly can't have any value stored in them).

Also you use sizeof wrongly. sizeof x tells you how many bytes are required to store x. (NOT anything that x may be pointing to, if x is a pointer, and not anything that x is a handle for, or a container class of). For example std::string is a container which has a pointer pointing to dynamically-allocated string contents. sizeof name only gets you the container size, it does not get you how much space has been dynamically allocated.

In your code, memsize can be determined at compile-time; it would never be different on different calls to the function.

Further, using a function CreateDIFDict is poor style as it forces dynamic allocation. Instead, provide a constructor. Then the user can choose whether they want dynamic allocation, and they can take advantage of move and copy semantics.

It's not clear whether or not you want DIFDict to take a copy of the strings and DIFColors that you are initializing it with. If you do not want a copy:

struct DIFDict
{
    std::string name;
    std::string *defs;
    struct DIFColor **colors;
    uint8_t defsize; 

    DIFDict(std::string name, std::string *defs, struct DIFColor **colors, uint8_t defsize) :
         name(name), defs(defs), colors(colors), defsize(defsize) {}
};

Make very sure that you destroy all DIFDicts before you destroy the things that the pointers are pointing to. (Raw pointers do not have an ownership semantic in C++).

If you do want a copy:

struct DIFDict
{ 
    std::string name;
    std::vector<std::string> defs;    // or some other container
    // ??? colors

    DIFDict(std::string name, std::string const *p_defs, uint8_t defsize)
         : name(name), defs(p_defs, p_defs + defsize) {}
};

I'm not sure what colors is supposed to point to, if you give more information about the expected behaviour of colors I'll update this example.

Upvotes: 2

jww
jww

Reputation: 102215

This is C++, not C. Drop the typedef and initialize it as you would a class. The only difference between a struct and a class is default visibility of the members. Something like:

struct DIFDict
{
    string name;
    vector<string> defs;
    DIFColor* colors;
    uint8_t defsize; 

    DIFDict();
    ~DIFDict();    
};

DIFDict::DIFDict : name(""), defs(...), colors(...), defsize(0) {}
DIFDict::~DIFDict() {...}

And as 0x499602D2 points out below, you should probably use a vector for your arrays.

The ellipses mean to create a helper to initialize the members. The helper should ensure the constructor behaves as expected during an exception.

Upvotes: 1

Related Questions