1

I'm new to structures so please bear with me. I wrote a structure called gnt containing an integer pointer, an integer, and a boolean:

struct gnt
{
    unsigned int* num;
    unsigned int size;
    bool negative;
};

Because I am allocating arbitrary length int arrays to various gnt variables (ie. k.num = new int*[j]) (for some value j), I need to free them somehow. I am not sure how to do that. Do I simply use delete[] k.num; (where k is a gnt)? Do I have to worry about the structure itself?

Also, as a side question, I wrote a recursive function to multiply out items in a list:

char* chain_multi(char** list, unsigned int start, unsigned int end)
{
    /***************************************************************
    This function recursively multiply every items in a list then
    return the product.
    ***************************************************************/
    unsigned int size = start - end + 1;
    if(size == 1)
        return copy_char(list[end]);
    if(size == 2)
        return multiplication(list[start], list[end]);

    int rs = start - size / 2;
    char* right = chain_multi(list, rs, end);
    char* left = chain_multi(list, start, rs + 1);
    char* product = multiplication(left, right);
    delete[] left;
    delete[] right;
    return product;
}

Will this give any advantage over doing it without recursion? I tested with various sized lists (between 10 - 10000 entries) and there doesn't seem to be any advantage time wise... The recursive code is shorter than its counterpart though.

Thanks for any input.

sth128
  • 93
  • 2
  • 8
  • 3
    Since the question is flagged as `c++`, why don't you just use `std::vector< int >`? Additionally, pointers are often a source of trouble, esp. inside classes/structures, since you may end up deleting stuff twice. – hochl Nov 09 '11 at 16:00
  • I was under the impression that vectors are slower than arrays? I am doing factorials of large numbers (ie. 100,000!) and faster is better. I am not allowed to use 3rd party libraries so no GMP, Xint, or MPIR or whatever, not that I can tell what those libraries are doing from the raw codes anyway. – sth128 Nov 09 '11 at 16:04
  • 2
    That depends, I suggest you use `std::vector< int >.reserve()` if it is possible, since you save yourself a **lot** of trouble using `vector`. If you feel it is too slow you should compare both implementations and see how big the speed difference really is. – hochl Nov 09 '11 at 16:09
  • you meant `k.num = new int[j]` right? Otherwise you have some bug. Also, std::vector is not noticeably slower in release builds, and is MUCH safer and easier to use than a naked pointer. – Mooing Duck Nov 09 '11 at 18:00
  • @MooingDuck: Yeah that's what I meant to type. Got confused after coding a bunch of char** (pointer pointer). Naked pointer? Is that the official jargon? Well then my program must look like some kind of pointer orgy LOL. – sth128 Nov 09 '11 at 23:49
  • @sth128: It's not official jargon, but it's common. There's smart pointers vs naked pointers, and containers vs naked arrays. – Mooing Duck Nov 09 '11 at 23:50

4 Answers4

3

Follow the Rule:
You should pass the same address to delete[] that you received from new[].
If You allocated only a member on freestore, so you would need to deallocate only that.

You allocated the member k.num using new [], so yes you should call delete [] only for it.

Also, You can use std::vector instead of doing the memory management by yourself(unless this is some crappy assignment which restricts you from doing so)


For Standerdese Fans:
Standard C++03 § 3.7.4.2-3:

If a deallocation function terminates by throwing an exception, the behavior is undefined. The value of the first argument supplied to a deallocation function may be a null pointer value; if so, and if the deallocation function is one supplied in the standard library, the call has no effect. Otherwise, the value supplied to operator delete(void*) in the standard library shall be one of the values returned by a previous invocation of either operator new(std::size_t) or operator new(std::size_t, const std::nothrow_-t&) in the standard library, and the value supplied to operator delete[](void*) in the standard library shall be one of the values returned by a previous invocation of either operator new[](std::size_t) or operator new[](std::size_t, const std::nothrow_t&) in the standard library.

Alok Save
  • 202,538
  • 53
  • 430
  • 533
  • It's not a homework. Or at least, not one assigned by someone. I wanted to learn this stuff so I set out the goals for this "assignment". But regarding memory management, is that an important concept to learn in C++ or should I just use vectors for everything? – sth128 Nov 09 '11 at 16:11
  • @sth128: Memory Management is an important concept and You must learn it.Also, C++ provides a number of data structures like `std::vector` which makes it very easy for the end user.But ofcourse not always.You should read more in to STL to understand the details.Repeating again,Yes its important. – Alok Save Nov 09 '11 at 16:15
  • You should understand all concepts to know why you're better of using `std::vector` :-) – hochl Nov 09 '11 at 16:15
  • @hochl: Yes yes, all concept understand must I. long way to go, I still have. Use the forks. – sth128 Nov 09 '11 at 16:38
3

Since you're using c++, you can put a destructor in the struct to do that automatically for you. There are other ways, but this is the most practical:

struct gnt
{
    unsigned int* num;
    unsigned int size;
    bool negative;
    ~gnt() {delete [] num; }
};

I'd also suggest to have a constructor to make sure that num has null until it's initialized, so the destructor will work safely before that:

struct gnt
{
    unsigned int* num;
    unsigned int size;
    bool negative;
    gnt() : num(NULL) {}
    ~gnt() {delete [] num; }
};

To have a safe behavior when instances are assigned or initialized when created, you need the copy constructor and assignment operator. They should copy the values of all the non-dynamic members, and create a duplicate of num with the same size and contents. In such case, it'd be also recommended to initialize all the members in the constructor, because size should also always have a valid content for that to work. If you don't want to complicate things too much now, just make them private, this will cause the compiler to bark if you try to do an (unsupported) object assignment or copy:

struct gnt
{
    unsigned int* num;
    unsigned int size;
    bool negative;
    gnt() : num(NULL) {}
    ~gnt() {delete [] num; }
    private: gnt(const gnt&); gnt &operator = (gnt &);
};

As others suggested, one alternative is to use std::vector instead of a raw pointer. That way, you don't need to worry about deallocations:

struct gnt
{
    std::vector<unsigned int> num;
    unsigned int size;
    bool negative;
};

About the question "do I have to worry about the structure itself?", that depends on how you created its instances. If it was with operator new, yes. If not, they'll be deallocated when goin out of scope as any other variable.

Finally, about the recursion, IMO rarely the choice is about code efficiency. You should use recursion only if the code becomes simpler/cleaner AND there is no danger of adverse effects (like stack overflow). If that's not the case, I'd always go for the iterative version.

Fabio Ceconello
  • 15,819
  • 5
  • 38
  • 51
  • 3
    Since you're at it, I suggest to initialize `num` as in `gnt(): num(0) { ... }`. Additionallty, as the structure contains a pointer, you **have to** implement the copy constructor and assignment operator as well if the structure will be copied (which is likely), or you end up deleting `num` more than once. – hochl Nov 09 '11 at 16:27
  • @Fabio: Wow thanks. Can I assume that by having the constructor and destructor inside the struct itself, I no longer have to manually call `delete[]` and that the variable will be automatically deallocated when going out of scope? – sth128 Nov 09 '11 at 16:27
  • @sth128: yes, exactly. The destructor will do the repetitive work automatically for you. – Fabio Ceconello Nov 09 '11 at 16:32
  • @hochl: you're correct, a 'finished' implementation should have that. I'm adding to my answer. Nice remark about the initializer list also. – Fabio Ceconello Nov 09 '11 at 16:33
1

The usual advantage of recursion is simplicity and clarity (and possibly a different approach to a problem), not normally speed. In fact, rather the opposite used to be true: recursive implementations tended to be noticeably slower than iterative ones. Modern hardware has eliminated or drastically reduced that speed differential, but it would still be fairly unusual for a recursive implementation to be faster than an iterative counterpart.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
0

free is fine on any pointer disregarding the fact it is a structure member or not. If you are writing C code maybe it would be better to use malloc() and free().

For recursion or not it depends on the context. Generally speaking recursion is ok. Recursion is slightly slower because of some function calling and parameter passing overhead. The problem with recursion is that if you go very deep in recursion level (maybe 1000 nested function call) you could end up filling the stack. This would cause the program to crash.

Paolo
  • 2,461
  • 5
  • 31
  • 45
  • The Q is tagged `C++` not `C`. – Alok Save Nov 09 '11 at 16:06
  • Ah that makes sense. I'm not too worried about this particular function reaching the stack limit though, as I understand it a stack limit of 1000 would allow this function to process 2^1000 entries no? That is way beyond the memory range of my computer. Heck, I think arrays can't go beyond 2^32 entries anyhow (on 32bit systems)? – sth128 Nov 09 '11 at 16:18
  • @sth128: Actually, I think they won't go beyond 2^INT_MAX elements, which is usually ~2.14e9, even on 64-bit systems. – Mooing Duck Nov 09 '11 at 23:52
  • I think you meant 2^31... Isn't `INT_MAX` something like 4 billion? (ie. 2^32) 2^INT_MAX would be an exceedingly large number, even greater than 2^1000... It would be over A BILLION digits in length. That number is greater than all the atoms in the universe multiplied by the age of the universe in Plank time! – sth128 Nov 10 '11 at 23:25