The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Learning C++ Question

I have a template container that is being used to hold pointers to an abstract base class. This base class has both virtual and non-virtual methods. One of the non-virtual methods makes use of two virtual methods to get it's job done. In the code I obtain a base class pointer from the container then call this non-virtual method and get a "pure virtual method called" error and the program aborts on the line that calls the virtual method (this->virtualmethod()). Searching for the answer on Google didn't turn up much so I'm resorting to asking. Any idea how you do this in C++, I do it in Java all the time.
Justin Kolb Send private email
Wednesday, September 08, 2004
 
 
http://c2.com/cgi/wiki?ObjectSlicing

(I haven't seen your code, so this is a guess)

Make sure you're not a victim of slicing. If your  insertion code looks like this:

void insert(T& item)
{
    //
    // assume data is an STL container,
    // all of which make a copy of the
    // item they are storing
    //
    data.push_back(&item);
}

or this:

void insert(T item)
{
    data.push_back(&item);
}

and not this:

void insert(T* item)
{
    data.push_back(item);
}


you are a victim of slicing (read the link for more details; I'd elaborate but my brain is fried right now. someone please correct me if I'm off).
Dan J
Wednesday, September 08, 2004
 
 
I'm using pointers and a hand coded container class (since I'm trying to learn). This code is used to access a B-Tree file where leaf nodes and internal nodes are slightly different from each other, so there are two subclasses to IndexRecord for each case, with common code for each in the base class.

<IndexRecord.h>
class IndexRecord
{
    public:
        IndexKey *findKey(unsigned long objectId);
        
        virtual void moveFirst() = 0;
        virtual IndexPosition moveNext() = 0;
};

<IndexRecord.cpp>
IndexKey *IndexRecord::findKey(unsigned long objectId)
{
    this->moveFirst(); // Error occurs here
    
    do
    {
        if (isAtKey())
        {
            IndexKey *key = getKey();
            
            if (key->objectId == objectId)
            {
                return key;
            }
            else if (key->objectId > objectId)
            {
                return 0;
            }
        }
    }
    while (this->moveNext() != END);
    
    return 0;
}

<LeafRecord.h>
class LeafRecord : public IndexRecord
{
    public:
    void moveFirst();
    IndexPosition movePrevious();
};

And here is the code that calls the above:

IndexKey *Index::findKey(unsigned long objectId, IndexRecord *indexRecord)
{
    IndexKey *key = indexRecord->findKey(objectId);
    push(indexRecord);
    
    if (key == 0)
    {
        if (indexRecord->isLeaf())
        {
            return 0;
        }
        else
        {
            return findKey(objectId, m_Cache->getNextIndex(indexRecord));
        }
    }
    else
    {
        return key;
    }
}
Justin Kolb Send private email
Wednesday, September 08, 2004
 
 
The IndexRecord class is an abstract class with 2 pure virtual methods. The derived class LeafRecord is also an abstract class, as the moveNext() method is not implemented and therefore remains pure virtual.

I don't know the context in which you call the Index::findKey method, but my guess is that you are trying to instantiate the LeafRecord class, which like I said, is abstract. If you did not paste all code, I might be wrong.

Careful with pure virtual methods. Using them forces restrictions on derived classes, which in some cases might be desired, but in others not.

I hope it helps.
Dany Send private email
Thursday, September 09, 2004
 
 
LeafRecord is implemented I just didn't include the implementation.
Justin Kolb Send private email
Thursday, September 09, 2004
 
 
Also the IndexRecord passed into findKey comes from a template class that stores IndexRecord pointers, but contains instances of LeafRecords and NodeRecords only. You can see farther down in findKey where it recursively calls itself after retreiving the next record instance from the template class.
Justin Kolb Send private email
Thursday, September 09, 2004
 
 
Yes, I think it's that "= 0" in the virtual method declaration that is giving you the trouble.  That makes the method 'Pure Virtual' -- meaning it MUST be overridden.  Being Pure Virtual, I believe you MUST instantiate an inheritor of this class -- you can't use the class directly.

If you left the "= 0" out, it would 'merely' be a virtual method.  You would then have the choice of declaring a local version of it, intended to be over-loaded by inheritors of the class.
AllanL5
Thursday, September 09, 2004
 
 
Check your constructors & non-virtual destructors. During construction & (non-virtual) destructions virtual functions act like non-virtual ones.
Dino
Thursday, September 09, 2004
 
 
I think the problem is that the IndexRecord::findKey method is _not_ virtual, while being defined in an abstract class. Therefore, everytime you will call this method, it will be called by an IndexRecord object, which which has no option but to call its own moveFirst method, which is a pure virtual one (also made explicit by calling this->moveFirst()).

Making IndexRecord::findKey a virtual method should solve your problem.
Dany Send private email
Friday, September 10, 2004
 
 
Sorry for saying stupid things. If that method is not virtual it only means that you cannot redefine the method in any derived classes and therefore you will not have the benefit of polymorphism. However, the method will be inherited in derived classes.

Here's a small test I wrote. Please let me know if there's any significant difference between it and your code.


#include <stdio.h>


class Abstract
{
public:
    void local() { printf("Abstract::local\n"); this->a(); this->b(); }

    virtual void a() = 0;
    virtual void b() = 0;
};


class Concrete : public Abstract
{
public:
    virtual void a() { printf("Concrete::a\n"); }
    virtual void b() { printf("Concrete::a\n"); }
};


class User
{
public:
    void caller(Abstract *object) { printf("User::caller\n"); object->local(); }
};


int main()
{
    Abstract *obj = new Concrete;

    User user;
    user.caller(obj);

    delete obj;
    return 0;
}


As a general rule, when you're faced with such problems it's easier to solve them if you write simple tests like this instead of more complicated code.

This code compiles fine with gcc 3.2 and runs as expected:

User::caller
Abstract::local
Concrete::a
Concrete::a
Dany Send private email
Friday, September 10, 2004
 
 
After studying it a long time last night I think it may be related to the template container the original reference comes from, because all the examples of calling a method like your example code and my code make it seem like it's no big deal and it should work and I cannot find any references to problems doing this. Usually when this happens it means I'm doing something right but something else is wrong.

I noticed last night in the debugger that when it gets to that line before the abort there is a hidden variable that contains the mangled name of the type and it has IndexRecord and no mention of NodeRecord or LeafRecord. This led me to believe that it's not getting a polymorphic reference from the container like it should which could be the case given my newness to the language.

Also I after looking at how the STL containers are created I saw that I wasn't matching how they handled the type parameter. So when I declare a containter of pointers using my class it looks like this

HashMap<IndexRecord>

while under STL it would look like this

HashMap<IndexRecord *>

So this could definitely be the culprit since I don't seem to be matching STL's way of doing things and since I'm not sure completely of the difference I'm going to implement the HashMap without templates specifically for pointers to IndexRecords then I'm going to see if that helps. If it does I might just end up going with an STL container since I don't feel like mucking around anymore with templates since it is obvious they are still a bit too advanced for me.
Justin Kolb Send private email
Friday, September 10, 2004
 
 
It's not only with STL that you have to declare an array of pointers in that way. It's with all generic types. STL just happends to have a nice generic implementation for its containers :-).

When a container is generic, it's generic all the way. There shouldn't be such a thing as a generic container of pointers, meaning that whatever you put inside it, it will be interpreted as a pointer to that whatever. From the container's point of view, it doesn't matter that you store objects of some type or pointers to objects of some type. He will treat both the same.

So, both variants you presented are legal, but they have different meanings. In your context, to benefit the joys of polymorphism, you'll need to use the second one ;-).

Templates are one of the most powerful features of C++. It's true that it's a more complicated feature, but it's certainly worth exploring. With generic containers you've barely scratched the tip of the iceberg :-).
Dany Send private email
Friday, September 10, 2004
 
 
You cannot instantiate abstract classes. In fact, the compiler catches abstract class instantiations.

Therefore your "call to a pure function" should not be possible unless virtual dispatching is disabled. And it is in the C++ standard to have virtual dispatching disabled during construction and destruction when the "this" pointer is invalid.

If you think about it, a constructor is a class level function (it should be static). Some languages have it this way (Pascal).

Destruction is instance related (hence you may have a virtual destructor) but once the destruction process starts the "this" becomes invalid. It is unclear if virtual calls can be made or not at that point.

But don't take my word for it, check the Stroustrup's C++ :-)
Dino
Friday, September 10, 2004
 
 
I poured over that book most of last night, it's a tough read (especially tough to find stuff you've already read in the index). I double checked that this wasn't happening during construction or destruction, so I figured either gcc has a bug or somehow I'm getting a pointer to this class that isn't marked as one of the derived classes. Given my low C++ knowledge the bad pointer seemed more likely which made me question how I was implementing my template container class. Once I get rid of the template and replace it with a type specific implementation I'll hopefully solve this problem and possibly learn what I did wrong in my original template class or find out that gcc has a bug =).
Justin Kolb Send private email
Friday, September 10, 2004
 
 
Make a simple sample fail. Then try it on different platforms.
Dino
Friday, September 10, 2004
 
 
Well that wasn't the problem, could someone take a look at my code and tell me what I'm doing wrong?

http://www.nutzy.net/NutzyEngine20040910.tar.gz

The pertinent files are:
Game.h & Game.mm (line 71 is the initial call that sets off the problem)
Index.h & Index.cpp
IndexRecord.h & IndexRecord.cpp (line 105 is where the error occurs)
LeafRecord.h & LeafRecord.cpp
NodeRecord.h & NodeRecord.cpp
IndexRecordHashMap.h & IndexRecordHashMap.cpp
PortalFile.h & PortalFile.cpp
IndexedFile.h & IndexedFile.cpp

Sorry you will not be able to run the code, it's Mac specific and also requires a data file from the game Asheron's Call.
Justin Kolb Send private email
Friday, September 10, 2004
 
 
Forgot the files IndexCache.h & IndexCache.cpp are also part of the code called.
Justin Kolb Send private email
Friday, September 10, 2004
 
 
Arrrrghhh! I created 3 test classes that replicate the hierarchy in the most simple way possible, and it works perfectly!!! =(

And C++ guys wonder why people don't like it!
Justin Kolb Send private email
Saturday, September 11, 2004
 
 
OK I found the problem. I am keeping a stack of pointers when searching the index and when I was clearing out the stack I was deleting all the pointers which were supposed to be controlled only by the cache. I guess this was a case of accessing a delete pointer. Sheesh only took me 3 days to track down.
Justin Kolb Send private email
Saturday, September 11, 2004
 
 
valgrind is a neat tool for finding such problems (memory access problems, memory leaks etc).

Saturday, September 11, 2004
 
 
You need an overloaded operator new and delete. Both should fill the about-to-be-allocated and about-to-be-freed memory with crap. That way, when/if you try to access deleted memory you will get something really weird rather than something that sometimes works and sometimes doesn't.

The incredibly C++-programmer-hostile Visual C++ provides this by default, but nothing else seems to. This is really weird, because this is, like, the single most important thing you need when programming in C or C++, after a source-level debugger, a working knowledge of your computer's assembly language, and a lot of patience.

The VC++ one is somewhat perfunctory, but despite its crapness has very often saved me endless hassle and heartache, though not quite enough to make up for the lack of a keyboard shortcut to close that fucking breakpoints window. (Valgrind sounds pretty cool, but is by no means mandatory, so don't fret if you can't use it. And anyway, it is as far as I can tell useless unless you use the system allocator for absolutely every single little last thing.)
Tom_
Saturday, September 11, 2004
 
 
Intersting, on windows & UNIX you would normally get a segment violation when accessing an invalid pointer.
Dino
Monday, September 13, 2004
 
 
I was wondering if there was a way that when deleting the pointers it wasn't deleting the base class part. That might explain the strange behavior.
Justin Kolb Send private email
Tuesday, September 14, 2004
 
 
If your destructor is non-virtual and you call the base class destructor then you an invalid delete (you delete the base).

The other way around, I don't know.
Dino
Tuesday, September 14, 2004
 
 
Check out boost::shared_ptr at

http://www.boost.org/

It's about to be included in the C++ standard, and is recommended over bare pointers for storage in containers as you described.  See also Herb Sutter and Jim Hyslop's article "Conversations:  Collecting Shared Objects" in the August 2004 C/C++ Users Journal.  http://www.cuj.com/
Mike Bland Send private email
Tuesday, September 21, 2004
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz