Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Alternative design to avoid dynamic_cast?

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?

like image 938
rr- Avatar asked Nov 28 '25 11:11

rr-


2 Answers

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.

like image 77
Petr Avatar answered Dec 01 '25 02:12

Petr


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
        }
    }
}
like image 20
Horstling Avatar answered Dec 01 '25 02:12

Horstling