About ownership of heap objects and C++ & pass-by-reference parameters

Question

I would like my class to be:

class NumberedString : public Object {
public:
    String newName;
    short nameID;
    NumberedString(String &newName, short nameID) : newName(newName), nameID(nameID) {}
};

HashMap uniqueStrs;//For later.

An instantiation of this will be passed to a HashMap which takes over ownership of its the heap allocation:

In HashMap.h (say):

virtual result Add(const Object& key, const Object& value);

Now this is where I get confused. I was allocating the String in the line that called Add:

uniqueStrs.Add(*(new String(L"XYX_say")), *pNewLoc);

The HashMap would then free this memory for me despite only accepting a reference to it. Maybe I lost a decade to C over the new millenium but I thought this was impossible?

If it's not then I should be able to write something like:

~NumberedString() {
    delete &newName;
}

for my class, but I'd never have guessed unless I saw this library HashMap::RemoveAll() doing the equivalent. This question states that this is impossible but falls back to reliance on auto_ptr and shared_ptr but my "platform supports only STL(Standard Template Library (http://www.sgi.com/tech/stl/))." (out of the entire "Standard C++ Library"). Could all answers please refrain from such references.

Thank you.

LINKS prompted by comments

I can't post the links as comments so please see the Add method and an example of its suggested use: here And Benj, String is not std::string no, sorry.

ALSO

I know it can cause crashes trying to delete stack objects but I don't get how HashMap can claim to be able to delete heap objects. I have coded up the above class to try and recreate this behaviour but I cannot accomplish the feat, hence the question.

In response to "Useless"

@Useless: Mightn't it be possible to pass to foo(int &bar) the variable *pBar, declared int pBar = new int(1); and then foo assumes ownership with

foo(int &bar) {
    int *__pBar = &bar;
    delete __pBar;
}

? I was going to try, but I am beginning to be cautious not to believe too much of what the documentation says. Though it was generated from the header which is saying

class _EXPORT_BASE_ HashMap :
    public IMap,
    public Object
    {
    virtual result Add(const Object& key, const Object& value);
        //other stuff
    };

Answer 1

Well, there's certainly nothing syntactically wrong with it. The only syntax rule for delete is that its operand has to be a pointer. Semantically: the pointer must be value returned from new, and that's where this idiom stinks; if I see a function taking a const reference, I'm normally justified in supposing that I can pass it a local variable, or a temporary, or such. In which case, the delete is going to cause really big problems.

More generally, having looked at the library documentation: I would avoid this library like the plague. It reminds me of a library from the NHS, which was widespread in the early days of C++: it requires that everything derive from Object, and containers contain Object*. Experience with this library back then (late 1980's) led to the conclusion that it didn't work, and were part of the motivation for adding templates to the language, so that we could write things that did work. Using this library is basically going back 25 years in time, and throwing out everything we've learned since then. (Java followed a similar route about 10 years later, so it's not something specific to C++. Basically, the solution proposed is one that was developed for languages with full dynamic type checking, like Lisp, Smalltalk or more recently Python, and doesn't work in languages with static type checking, like C++ or Java.)

Answer 2

uniqueStrs.Add(*new String(L"XYX_say"), *pNewLoc);

_{Removed the extra parentheses, which were wrong; I guess you didn't want to ask about them.}

The HashMap would then free this memory for me despite only accepting a reference to it. Maybe I lost a decade to C over the new millenium but I thought this was impossible?

It is possible, and delete &newName; is legal, given that newName is actually a result of *new .... However, it is unidiomatic especially with the decaration

virtual result Add(const Object& key, const Object& value);

Since it takes its arguments as const-references, it can also take rvalues implicitly converted to const references:

uniqueStrs.Add(String(L"XYX_say"), something)

This will lead to crashes (because the rvalue ceases to exist after the call, because the delete will delete a non-heap allocated object etc.) but the interface doesn't clearly show it and it is customary to pass rvalues to functions taking const-references.

Answer 3

If you have:

class NumberedString : public Object {
    String newName;
    ...
};

compiler generates NumberedString's destructor which automatically calls destructors of all member objects, including newName. Therefore you don't need to do something like this (which doesn't make sense anyway):

~NumberedString() {
    delete &newName;
}

I had a look at Bada API which for HashMap::Add(const Object& key, const Object& value) says:

This method performs a shallow copy. It adds only the pointer; not the element itself.

This interface is a bit misleading and potentially dangerous - jpalacek explained what can happen if you pass object which is not on the heap. In my opinion, this function should have pointer types as arguments, that would be much clearer.

For HashMap::Remove(const Object& key, bool deallocate = false) and HashMap::RemoveAll(bool deallocate = false) documentation says:

Removes all the object pointers in the collection. If the deallocate param is true, it also removes all of the objects.

So, by default these functions will just remove pointers but your objects will still be alive.

In your current implementation NumberedString is responsible for the lifetime of its members. When instance of that class is destroyed, its members will be destroyed. If you pass its members to HashMap, delete your object/remove it from the stack and then call HashMap::RemoveAll(false), HashMap won't try do deallocate objects twice. Be aware that after deleting your object HashMap will be holding pointers to deallocated memory (dangling pointers) which is dangerous. If you call HashMap::RemoveAll(true), HashMap will try to deallocate memory that has already been deallocated, which is dangerous as well.

Better design could be to have something like this:

class NumberedString : public Object {
    String* pNewName;
    short* pNameID;
    ...
}

where you make sure that NumberedString doesn't own String and short objects (doesn't delete them in its destructor). This class would be keeping pointers, just as HashMap. You will need to define who's creating and who's destroying these objects. E.g. you could create them, pass their addresses to NumberedString and HashMap and then delegate HashMap to delete them by calling HashMap::RemoveAll(true). Or, you can delete them yourself and then call HashMap::RemoveAll(false).

The conclusion is: be very careful with this silly API and pay attention to when and by whom are your objects created and deleted.