views:

52

answers:

2

I'm implementing a Zip-Wrapper (zlib minizip) and asking myself how i should handle exceptions properly. I'm thinking of three versions. Which one would you prefer, or is there a version i didn't thought about?

The task of the function Install is to get a Zip-File from a Web-Server, unpack its content and delete the downloaded Zip-File. But if an error occurs while unpacking the file, where should the Zip-File be deleted?

Tanks for your experiences.

Version A (with deleting outside the function):

void Install() {
    getFile("upd.zip"); // Creates File
    MyZip myzip("upd.zip");
    myzip.unzip();      // Can't do its job --> Exception
    delete("upd.zip");  // In case of exception: File would not be deleted here
}

int main() {
    try {
        Install();
    }
    catch (const Error& err) {
        delete("upd.zip"); // File must be deleted here
        MessageBox(err.text);
    }
}

Version B (with Re-Throwing the exception)

void Install() {
    getFile("upd.zip"); // Creates File
    try {
        MyZip myzip("upd.zip");
        myzip.unzip();
    }
    catch (const Error& err) {
        delete("upd.zip");
        throw err; // Re-Throw the Error
    }
    delete("upd.zip");
}

int main() {
    try {
        Install();
    }
    catch (const Error& err) {
        MessageBox(err.text);
    }
}

Version C (with return code)

void Install() {
    getFile("upd.zip"); // Creates File
    MyZip myzip("upd.zip");
    if (!myzip.unzip("upd.zip")) {
        delete("upd.zip");
        throw Error(myzip.geterror()); // what was the reason
    }
    delete("upd.zip");
}

int main() {
    // Same as in Version B
}
+3  A: 

None of the three. Use RAII:

class FileGuard {
public:
    FileGurad(const std::string & filePath)
    : m_FilePath( filePath )
    {}

    ~FileGuard() {
        delete(m_FilePath); // must not throw an exception
    }
private:
    std::string m_FilePath;
};

Usage:

void Install() {
    guard FileGuard(getFile("upd.zip")); // Creates File; getFile should return the path where the file was created
    MyZip myzip("upd.zip");
    myzip.unzip();      // Can't do its job --> Exception
    // file will be deleted when guard goes out of scope (e.g. when an expection is thrown)
}

Alternatively you can have FileGuard call getFile in the constructor. See this answer (and others to the same question) for more information about stack unwinding (esp. the order of destruction).

Space_C0wb0y
Thanks for the answer. I did it this way. Is the order for destroying class instances always the same (First created = last destroyed)? I ask because in the sense of RAII i gave the part for closing the zipfile within the destructor of MyZip. Thus the MyZip instance should be destroyed before the FileGuard instance.
Christian Ammer
@Christian: I added a link for your question about the order.
Space_C0wb0y
A: 

I would go with version B. Edit: Ofcourse RAII is desirable but given the above 3 would go for version B

Als