Clever Geek Handbook

Alternative design to avoid dynamic_cast?

Let's say I have an interface Archive

and File

.

  • Everyone is File

    guaranteed the availability of at least std::string name

    .
  • Everyone Archive

    can have std::vector<File*> Archive::list() const

    his files.
  • Everyone Archive

    can Archive::extract(std::vector<File*> files)

    .

Then I have ZipArchive

and ZipFile

, ZipFile

contains the archive offset and other implementation details. Then there is TarArchive

/ TarFile

, etc. Each of them fills with std::vector<File*> list() const

instances ZipFile

, TarFile

etc.

list()

is intended to give users the ability to select files to unpack. They pick elements from this vector, then pass that vector to extract()

.

At this point ZipArchive

it is necessary to assume that it passed the correct type and dynamic_cast<ZipFile*>(file)

to access the implementation details.

This is bad. It is acceptable? Are there alternatives?

+3


source to share


4 answers


As pointed out in the comments, you can move the retrieved interface from Archive

to File

. The archive will be returned std::vector<File*>

, but in reality each object will be, for example, ZipFile

and will know to which archive it belongs and what its type is, and will be able to call the correct extraction method.

As a result, you can have code without checking the 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 complicated if you want to extract many files in one call, but you can have an archive extractFile

, just remember this file, and then a special method in the class Archive

to extract all memorized files in one go. I think it might even be hidden under a fairly simple interface.

+2


source


Yours ZipArchive

could search in your file list for the passed pointer. If it's there, it can either use a stored pointer (which already has a type ZipFile

) or a static_cast

passed pointer to ZipFile

(because you've proven its type). If the passed pointer is not in the list, it is clearly not a file belonging to this archive, so you can continue to handle errors.

You can also add a reverse type pointer Archive*

to each File

. A concrete implementation ZipArchive

can check if its one of its files is not a simple pointer compass.



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
        }
    }
}
+1


source 


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 implementation ZipArchive

in a method, extract

you can do a static_cast rather than a dynamic one if the int returned archiveType

is the same as the name 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
        }
    }
}
0


source


You need to analyze how you will treat Archive

s: do you need to have some general behavior in some places due to undefined types, or not? This will lead to two different designs, so choose carefully.

As said in the comments, you don't seem to need the former.
Let File

is a file descriptor, and ZipFile

, TarFile

- its derivatives. Then, for each type of file, let it Archive

process 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

. No more need to handle property rights, pointers, etc .; you also get strong security.

0


source






More articles:


All Articles