views:

67

answers:

1

I have a class designed to do import/export of data in one of a few different formats. Each format should have exactly the same interface, so I'm implementing it as a base class with a bunch of virtual methods and a derived class for each specific format:

#ifndef _IMPORTEXPORT_H_
#define _IMPORTEXPORT_H_

#include "stdio.h"

enum EXPORT_TYPE {
    EXPORT_INI = 1,
};

class exportfile {
public: 
    virtual ~exportfile();

    static exportfile * openExportFile(const char * file, EXPORT_TYPE type);

    virtual void startSection(int id) = 0;
    virtual void endSection() = 0;

protected:
    exportfile(const char * file);
    FILE * hFile;

};

class iniexportfile : public exportfile {
public:
    iniexportfile(const char * file) : exportfile(file) { }

    void startSection(int id);
    void endSection();
private:
    bool inSection;

};

#endif

This is the base class (exportfile) and one of the derived classes (iniexportfile).

The implementation of these methods are as such:

#include "importexport.h"

#include <exception>
#include <assert.h>

exportfile * openExportFile(const char * file, EXPORT_TYPE type) {
    switch(type) {
        case EXPORT_INI:
            return new iniexportfile(file);
        default:
            return NULL;
    }
}

exportfile::exportfile(const char * file) {

        this->hFile = fopen(file, "w");

        if(this->hFile == 0) {
            throw new std::exception("Unable to open export file");
        }
}

exportfile::~exportfile() {
    assert(this->hFile != 0);

    this->endSection();

    fclose(this->hFile);
    this->hFile = 0;
}

void iniexportfile::startSection(int id) {
    assert(this->hFile != 0);

    fprintf(this->hFile, "[%d]\r\n", id);

    this->inSection = true;
}

void iniexportfile::endSection() {
    this->inSection = false;
}

(Note, the class is obviously not complete.)

Finally, I have a test method:

#include "importexport.h"

#include <exception>

using namespace std;

void runImportExportTest() {
    iniexportfile file("test.ini");

    file.startSection(1);

    file.endSection();
}

Anyway, this all compiles fine, but when it gets linked, the linker throws up this error:

error LNK2001: unresolved external symbol "public: virtual void __thiscall exportfile::endSection(void)" (?endSection@exportfile@@UAEXXZ)   importexport.obj

Why is it looking for exportfile::endSection(), when it's marked as pure virtual? Did I somehow not make it pure virtual? Or, have I been spoiled by C#, and am totally mucking these virtual functions up?

Incidentally, this is Visual Studio 2008. Thought I should mention that somewhere.

+2  A: 

By the time this dtor gets called:

exportfile::~exportfile() {
    assert(this->hFile != 0);

    this->endSection();

    fclose(this->hFile);
    this->hFile = 0;
}

the compiler has 'unwound' the vtable so it will resolve to the exportfile::endSection() function - it will not call the derived version. You'll need to design a different clean up method.

See "Never call virtual functions during construction or destruction".

Michael Burr
*smacks head* Yeah, you're right. That call should go in `iniexportfile::~iniexportfile`. It works fine when I do that.
Mike Caron
Of course, if I were to strictly say "no virtual methods in my destructors", how would I do this. Extract it into a non-virtual method, and call that from my destructor?
Mike Caron
@Mike: the thing is there are times when it would be nice to do what you wanted to do there. But I'm not sure of a better pattern than putting the required clean-up code in each most-derived class.
Michael Burr
@Mike: calling a non-virtual method is OK, but you won't be able to do what I think you're looking to do - by the time the base destructor is called, there's no more 'derived object' bits to work with, so the non-virtual function can't do anything that the base destructor couldn't do.
Michael Burr
@Michael: I meant in the derived classes. So, `private void iniexportfile::cleanupatdestruction()` or something sanely named. But, I'll just leave it in the destructor, since I know what's going on. (actually, I could just omit it, since it's a nop, but that's entirely beside the point)
Mike Caron