Search code examples
c++pointersmultimap

Using pointer to class objects as key in a multimap example


I've read a lot, but I can't seem to understand how this should work. This program was originally using a member multimap<CFile, Filetype>, but I need to remake it as multimap<CFile*, Filetype>. From my small understandings about pointers I need to make a multimap, which has pointers to CFile objects as key, but I fail at implementing this. I actually put pointers inside the multimap in the CDirectory constructor, but when I try to print an object of CDirectory the program crashes, so one (of many mistakes) is my overloading ostream operator<< for the CDirectory class. I really don't know where to even begin at, I get the basic pointer logic, but it seems that I can't implement it.


class CFile {
    string m_strFile;
    unsigned int m_size;
public:
   /*constructors, get() and set() functions are
     implemented but I've deleted them for the example */

    bool operator< (const CFile& obj) const {
        return (m_strFile < obj.m_strFile);
    }
    bool operator== (const CFile& obj) const {
        return (m_size == obj.m_size);
    }
    friend ostream& operator<< (ostream& ost, const CFile& obj) {
        return ost << "name: " << obj.m_strFile << ", size: " << obj.m_size;
    }
    friend istream& operator>> (istream& ist, CFile& obj) {
        return ist >> obj.m_strFile >> obj.m_size;
    }
};

class CDirectory {
    string m_strDirectory;
    enum class Filetype {
        Archive, Hidden, ReadOnly, System, FileNotSupported
    };
    multimap <CFile*, Filetype> m_DirectoryMap;
public:
    /* overloading operator<< for class CDirectory and uses the friend function filetypeToString to convert the enum value to string */
    friend std::ostream& operator<<(std::ostream &os, const CDirectory &dir) {
        os << dir.m_strDirectory << "\n";
        auto p = m_DirectoryMap.begin();
        while ( p != m_DirectoryMap.end()) {
           os << p->first->getFileName() << '\t' << p->first->getFileSize() << '\t' <<  CDirectory::filetypeToString(p->second) << '\n';
           ++p;
        }
        return os;
    }
    /* comparator function, used to find the min and max files by size
       - takes 2 pairs of the multimap and compares their CFile objects filesize */
    static bool Greater(const pair<const CFile, Filetype>& a,
                    const pair<const CFile, Filetype>& b) {
    return (a.first.getFileSize() < b.first.getFileSize());
    }
    /* explicit constructor - reads data from a file and inserts pairs
      of types pair <CFile, enum Filetype> in a multimap */
    CDirectory (const string& n) {
        fp.open (n, ios::in);
        if (!fp) {
            throw std::runtime_error("Could not open file");
        }
        string dirName, fileName,  fType;
        int fileSize;
        Filetype filetype;
        fp >> dirName;
        m_strDirectory = dirName;
        while (fp >> fileName >> fileSize >> fType) {
            CFile obj (fileName, fileSize);
            CFile* ptr = &obj;
            if (fType == "Archive")
                filetype = Filetype::Archive;
            else if (fType == "Hidden")
                filetype = Filetype::Hidden;
            else if (fType == "ReadOnly")
                filetype = Filetype::ReadOnly;
            else if (fType == "System")
                filetype = Filetype::System;
            else
                filetype = Filetype::FileNotSupported;
            m_DirectoryMap.insert(pair<CFile*, Filetype>(ptr, Filetype(filetype)));
        }
    }
    string getDirectory () const { return m_strDirectory; }
    void printMap () {
         auto p = m_DirectoryMap.begin();
         cout << m_strDirectory << endl;
         while ( p != m_DirectoryMap.end()) {
                cout << endl << p->first->getFileName() << '\t' << p->first->getFileSize() << '\t' << filetypeToString(p->second) << endl;
                ++p;
        }
    }
    int countDuplicates( const string& strToCount ) const {
        CFile obj (strToCount, 0);
        CFile* ptr = &obj;
        int numberOfDuplicates = m_DirectoryMap.count(ptr);

        if (numberOfDuplicates > 1)
            return numberOfDuplicates;
        else if (numberOfDuplicates == 1)
            return 1;
        else
            return 0;
    }
     void removeDuplicates( const string& strToRemove ) {
        CFile obj (strToRemove, 0);
        CFile* ptr = &obj;
        pair <multimap<CFile*,Filetype>::iterator, multimap<CFile*,Filetype>::iterator> range;
        range = m_DirectoryMap.equal_range(ptr);
        auto it = range.first;
        ++it;
        while (it != range.second)
            it = m_DirectoryMap.erase(it);
    }
 /*   CFile findMaxSize() const {
        multimap<CFile*, Filetype>::const_iterator result;
         result = std::max_element(m_DirectoryMap.begin(), m_DirectoryMap.end(), Greater);
         CFile obj(result.first->getFileName(), result.first->getFileSize());
         return obj;
    }
    CFile findMinSize() const {
        multimap<CFile*, Filetype>::const_iterator result;
        result = std::min_element(m_DirectoryMap.begin(), m_DirectoryMap.end(), Greater);
        CFile obj(result.first->getFileName(), result.first->getFileSize());
        return obj;
    } */
    static std::string filetypeToString(Filetype type) {
        switch (type) {
            case Filetype::Archive:
                return "archive";
                break;
            case Filetype::Hidden:
                return "hidden";
                break;
            case Filetype::ReadOnly:
                return "read-only";
                break;
            case Filetype::System:
                return "system";
                break;
            case Filetype::FileNotSupported:
                return "not-supported";
                break;
        }
    }
};

int main () {
    /* - Catching if the file exists, prompt the user untill a correct name is given
       - Making an object of type CDirectory, reading data from a file and inserting
       pairs of <CFile, Filetype> in a multimap.
       - Counting the number of duplicated filenames and removing them (leaving only 1).
       - Finding the min and max files by size and printing their objects. */
    string filename = "";
    int numberOfDuplicates = 0;
    cout << "Please enter input file name: \n";
    string iname = "";
    bool done = false;
    CDirectory obj();
    while (!done && cin >> iname) {
        ifstream ist{iname};
        try {
            CDirectory obj(iname);
            done = true;

            cout << "The original multimap (ordered by filename) contains the following data: \n\n";
            system("pause");
            cout << obj;

            cout << "\n\nCheck if the file has any duplicates. Enter a filename:\n\n";
            do  {
                cin >> filename;
                numberOfDuplicates = obj.countDuplicates(filename);
                if ( numberOfDuplicates > 1) {
                    cout << "The file has " << numberOfDuplicates << " duplicates.";
                    cout << "Removing duplicates of " << filename << ". \n\n";
                }
                else if (numberOfDuplicates == 1)
                    cout << "The file " << filename << " does not have any duplicates.\n\n";
                else if (numberOfDuplicates == 0)
                    cout << "The file " << filename << " is not in the multimap. Please enter a new filename:\n\n";
            } while (!(numberOfDuplicates > 0));

            system("pause");
            obj.removeDuplicates(filename);
            cout << "The updated multimap (ordered by filename) contains the following data: \n\n";
            cout << obj;

        }  catch (std::exception &ex) {
            std::cout << ex.what() << "!\n" << "Please try again.\n";
            }
    }
    getch();
    return 0;
}

Solution

  • In

    while (fp >> fileName >> fileSize >> fType) {
        CFile obj (fileName, fileSize);
        CFile* ptr = &obj;
        if (fType == "Archive")
            filetype = Filetype::Archive;
        else if (fType == "Hidden")
            filetype = Filetype::Hidden;
        else if (fType == "ReadOnly")
            filetype = Filetype::ReadOnly;
        else if (fType == "System")
            filetype = Filetype::System;
        else
            filetype = Filetype::FileNotSupported;
        m_DirectoryMap.insert(pair<CFile*, Filetype>(ptr, Filetype(filetype)));
    }
    

    You create a loop local variable obj with

    CFile obj (fileName, fileSize);
    

    Then you store a pointer to that object with

    CFile* ptr = &obj;
    //...
    m_DirectoryMap.insert(pair<CFile*, Filetype>(ptr, Filetype(filetype)));
    

    Then you start the loop over again. Unfortunately when you reach the end of the loop all loop objects are destroyed and then they are recreated when the loop starts back at the beginning. That means the map now has a pointer to an object that was destroyed. This pointer is no longer valid and using it is undefined behavior.

    A quick fix would be to create a pointer instead of an automatic object like

    CFile* obj = new CFile(fileName, fileSize);
    

    And then you store that pointer in the map. This does mean that you need to clean up the memory you allocated when you are done with the map. You would need to iterate through the map and call delete on every key.

    We then have another issue that when using

    multimap <CFile*, Filetype> m_DirectoryMap;
    

    The map is going to sort the keys by the address the pointer holds and not the pointed to CFile. To have the map sort by the actual CFile you need to write a custom comparitor that will take two CFile* and returns the comparison of the of the pointed to CFiles.