Alternative design to avoid dynamic_cast?

Question

Say I have Archive interface and File interface.

• Each File is guaranteed to have at least std::string name.
• Each Archive can std::vector<File*> Archive::list() const its files.
• Each Archive can Archive::extract(std::vector<File*> files).

Then I have ZipArchive and ZipFile, ZipFile contains offset in the archive file and other implementation details. Then there's TarArchive/TarFile and so on. Each of these fills std::vector<File*> list() const with instances of ZipFile, TarFile etc.

list() is meant to give users a chance to select which files to unpack. They select elements from that vector, then they pass this vector to extract().

At this point, the ZipArchive needs to assume it was passed the right type and do dynamic_cast<ZipFile*>(file) to access implementation details.

This feels bad. Is this acceptable? Are there alternatives?

Answer 1

Your ZipArchive could search in its list of files for the passed pointer. If it's in there, it could either use the stored pointer (which already is of type ZipFile) or static_cast the passed pointer to ZipFile (because you have proven its type). If the passed pointer is not in the list, it's obviously not a file owned by this archive, so you can go on with error handling.

You could also add a backpointer of type Archive* to every File. The concrete ZipArchive implementation can than check if its one of its files by a simple pointer comparsion.

void ZipArchive::extract(std::vector<File*> files) 
{
    for (auto file : files)
    {
        if (file->archive() == this) 
        {
            // one of my files
            auto zipFile = static_cast<ZipFile*>(file);
            // do something with zipFile
        }
        else
        {
            // file is owned by some other archive
        }
    }
}

Answer 2

class Archive { 
public:
    static int registerArchiveType(const std::string &name) {
        // generate a unique int for the requested name archive type
        // and store it in a map or whatever
        return uniqueInt;
    }

    int archiveType() const;

protected: 
    Archive(int type) : _type(type) {}
private: 
    int _type;

public:
     virtual extract(std::vector<File*> files);

    // your implementation details
};

class File {
public:
    int archiveType() { return _archType; }
protected:

    // force implementations to pass the same type
    // they received from the call to Archive::registerArchiveType
    File() {}
    void setArchiveType(const std::string &archiveType) {
        // multiple calls to registerArchiveType return the
        // same identifier if passed the same string
        _archiveType = Archive::registerArchiveType(archiveType);
    }

private:
    int _archiveType;
};

Then in your ZipArchive implementation, in the extract method you can perform a static_cast rathern than a dynamic one if the int returned by archiveType is the same as the one registered for the Zip archive type.

static const char* ZIP_TYPE = "zip";

// specialize your ZipFile making sure
// you pass the correct archive type identifier
// in the constructor
class ZipFile {
public:
    ZipFile() : File() {
        setArchiveType(ZIP_TYPE);
    }
    // bla bla
};

void ZipArchive::extract(std::vector<File*> files) {
    for (int i = 0; i < files.count(); i++) {
        if (files[i]->archiveType() == Archive::registerArchiveType(ZIP_TYPE)) {
            ZipFile *zipFile = static_cast<ZipFile*>(files[i]);
            // do something with zipFile
        }
    }
}

Answer 3

As suggested in the comments, you can move the extracting interface from Archive to File. The archive will return std::vector<File*>, but in fact each object will be, e.g., ZipFile, and will know which archive it belongs to and what is its type, and will be able to call proper extract method.

As a result, you can have code without any checking of archive type:

struct File;
struct Archive {
    virtual std::vector<File*> fileList() = 0;
};

struct File {
    File(std::string name_) : name(name_) {}
    virtual void extract() = 0;
    std::string name;
};

struct ZipFile;
struct ZipArchive: public Archive {
    void extractFile(ZipFile& file);
    virtual std::vector<File*> fileList();
};

struct ZipFile: public File {
    ZipArchive* archive;
    virtual void extract() { archive->extractFile(*this); }
    ZipFile(std::string name_, ZipArchive* archive_) : File(name_), archive(archive_) {}
};

Full example: http://ideone.com/kAs5Jc

It can be more diffucult if you want to extract many files with one call, but you can have archive's extractFile just remember that file, and then a special method in the Archive class to extract all the remembered files at once. I think this can even be hidden under a rather simple interface.

Answer 4

You need to analyze the way you will treat Archives: do you need to have in some places some common behavior due to undetermined types or do you not? This will bring to two different design choices, so choose carefully.

---

As said in the comments, you don't seem to need the former.
Let File represent a file handle and ZipFile, TarFile be derivations thereof. Then, for each type of file let an Archive handle it.

struct ZipFile 
{        
     File handle; 
     // zip-specific implementation details 
};

struct TarFile
{ 
     File handle;
     // tar-specific implementation details
};

class ZipArchive
{
     public:
            std::vector<ZipFile> list() const; 
            void extract(std::vector<ZipFile>);         

     private:
             std::vector<ZipFile> archive;
};

And the same for TarArchive. There is no more need to handle ownership, pointers and so on; you also get strong type safety.