Iterating over a list of objects c++

Question

Basically I am creating a basic shell program for a class project. I am stuck on iterating through the files/folders within a directory.

My error is in the function DisplayDirectoryContents()

class Directory 
{
private:
map<string, pair<list<Folder>, list<File> > > directoryContents;
string directoryPath;

public:
list<Folder> getFolders() {return directoryContents[directoryPath].first;}
list<File> getFiles() {return directoryContents[directoryPath].second;}
string getDirectoryPath() {return directoryPath;}

Directory() 
{
    directoryPath = "root/"; 
    File *file = new File("Test");
    directoryContents[directoryPath].second.push_back(*file);
}
void DisplayDirectoryContents()
{
    // Get files and folders from directory
    list<File> files = this->getFiles();

    for(int i = 0; i < files.size(); i++)
    {
        cout << files->getFileName(); << Error here
    }

}
};

I've tried setting this function up a few different ways but I can't seem to get the right kind of code to make it work.

I would think this would work but it's throwing me errors everytime I try and mess with the DisplayDirectoryContents() function. Does anyone have any tips that I can try to help fix this? Thanks

class File
{
string fileName;
string fileTime;
public:
string getFileTime(){return fileTime;}
string getFileName(){return fileName;}

void setFileTime(){time_t now = time(NULL);
                fileTime = ctime(&now);} 
void setFileName(string newFileName){fileName = newFileName;}
File(){}
File(string fName)
{
    fileName = fName;
    time_t now = time(NULL);
    fileTime = ctime(&now);
}
void MakeFile(string fName);
void RemoveFile(string fName); 
};

The map in your design just looks obsolete - instead of that map with just one single entry, just have two simple list members. — Aconcagua
– Aconcagua, Commented Sep 6, 2018 at 4:37
I do not see any definition for Folder - did you intend std::list<Directory> instead (would make sense at least...)? — Aconcagua
– Aconcagua, Commented Sep 6, 2018 at 4:39
So, the reason I used the map is because I am going to make it so you can add a lot more directories, files and folders. Should I still use it for that or switch to simple list members? — Jake Young
– Jake Young, Commented Sep 6, 2018 at 4:39
And yeah I was just testing to make sure it worked with the file before I moved onto the folder class — Jake Young
– Jake Young, Commented Sep 6, 2018 at 4:40
You can add arbitrary number of files or folders to the lists already. The map only is meaningful if you want to add entries for more than one different directory path. Directory and Folder will get two different classes for the same thing. Nothing speaks against adding other Directory instances to the parent Directory instance. Wouldn't this better reflect file systems as well? — Aconcagua
– Aconcagua, Commented Sep 6, 2018 at 4:47

R Sahu · Accepted Answer · 2018-09-06 04:33:27Z

2

If you are able to use C++11 or higher, you can use a range for loop to iterate over the contents of the list.

void DisplayDirectoryContents()
{
    // Get files and folders from directory
    list<File> files = this->getFiles();

    // Iterate over each item in the list.
    // Use a reference to avoid copying of the items.
    for ( File& file : files )
    {
        cout << file.getFileName();
    }
}

answered Sep 6, 2018 at 4:33

R Sahu

207k15 gold badges168 silver badges291 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Jake Young Over a year ago

Awesome! Yeah I am able to use it, and this worked! Appreciate the help.

Aconcagua · Accepted Answer · 2020-12-15 20:04:13Z

Extension to R Sahu's answer:

The reason you cannot iterate using your original loop is because std::list does not offer an index operator (you would need to use std::vector instead).

Alternative approach to the range based for loop (which I'd prefer, though, unless you have specific reasons not to do so) – or the way to go if you have no C++11 available – is using iterators:

for(std::list<File>::iterator i = files.begin(); i != files.end(); ++i)
{
    std::cout << i->getFileName();
}

With C++11, you can have for(auto i = files.begin(); ....

Use cases for using iterator loop even with C++11 can be comparing items with their successors:

// check for empty list first, as std::prev would fail on!)
for(auto i = files.begin(); i != std::prev(files.end()); ++i)
{
    if(someCondition(*i, *std::next(i))
    {
        // do something else
    }
}

There is already std::remove_if for and you should prefer it (together with a lambda; but don't forget to apply erase afterwards, see erase-remove-idiom!), so just for illustration; starting with the simpler approach:

for(auto i = files.begin(); i != files.end(); ) // no ++i (!)
{
    if(condition)
    {
        i = files.erase(i);
        // fine for std::list; with std::vector, if removing more
        // than one single element, we'd move or even copy subsequent
        // elements multiple times, which is quite inefficient
    }
}

Improved variant (which is what std::remove_if does as well):

auto pos = files.begin();
for(auto i = files.begin(); i != files.end(); ++i)
{
    if(!condition)
    // ^ (!)
    {
        *pos++ = std::move(*i);
    }
}
// here, std::remove_if already stops, but returns pos...
files.erase(pos, files.end());

A minimal problem might remain with move or copy assignment operators not handling self-assignment correctly, which would be covered by if(!condition && i != pos).

Collectives™ on Stack Overflow

Iterating over a list of objects c++

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related