CODEWITHSUNDEEP
listfor-loopc++11stliterator
user15687371
user15687371

Reputation:

C++ change list while iterating?

In C++11 have a list called jobs, in which I want to delete all jobs with stopped flag being true, so I wrote:

auto job = jobs.begin();
while (job != jobs.end()) {
    if (!job->stopped) {
        job = jobs.erase(job)
    } else {
        ++job;
    }
}

But someone took a look at my code and said it's wrong which I doin't understand why?

Upvotes: 1

Views: 144

Answers (1)

Vlad from Moscow
Vlad from Moscow

Reputation: 311048

If do not take into account the typo relative to the missing semicolon in this statement

job = jobs.erase(job)
                    ^^^

and the second typo in this condition

if (!job->stopped) {
    ^^^^

which should be written like

if ( job->stopped) {

(that is you need to remove as you wrote all jobs with the set flag stopped) your code is correct but is redundant.

You could just write

jobs.remove_if( []( const auto &job ) { return job.stopped; } );

or

jobs.remove_if( []( const JobEntry &job ) { return job.stopped; } );

if this statement is called within a member function.

Edit: Here is a demonstrative program that uses your class declarations.

#include <iostream>
#include <string>
#include <list>
#include <ctime>

typedef int pid_t;

class JobsList {
public:
    class JobEntry {
    public:
        pid_t pid, jid;
        std::string cmd;
        time_t in_time;
        bool stopped;

        JobEntry( int pid, int jid, const std::string &cmd, bool stopped )
            :pid( pid ), jid( jid ), cmd( cmd ), stopped( stopped )
        {}
        // TODO: Add your data members

        bool operator<( JobEntry const &tmp ) const {
            return jid < tmp.jid;
        }

        bool operator==( JobEntry const &tmp ) const {
            return jid == tmp.jid;
        }
    };
    std::list<JobEntry> jobs;
};

int main()
{
    JobsList jobs_list =
    {
        {
            { 1, 1, "first", false },
            { 2, 2, "second", true }
        }
    };

    std::cout << jobs_list.jobs.size() << '\n';
    jobs_list.jobs.remove_if( []( const auto &job ) { return job.stopped; } );
    std::cout << jobs_list.jobs.size() << '\n';
}

I only introduced for simplicity this typedef

typedef int pid_t;

and changed a parameter declaration in the constructor

JobEntry( int pid, int jid, const std::string &cmd, bool stopped )
                            ^^^^^^^

Upvotes: 3

Related Questions