Cannot add more than one struct onto a qlist within foreach loop

CybeX

About:

p.s.I know this is a specific question, but I am at a loss, and have no idea why this is happening

The algorithm below should add a struct to a list. Each struct is a server_connection object, and the receiving list, is contained within another struct named VPN_Server.

When adding the struct to the list, it holds only the very first struct added, and does not add any more.

This is confirmed by the debugger window:

for each new IP, a new VPN_Server struct is created and a new server_connection is created and push_back'ed onto the ServerInstancesList list. But when in the foreach loop, attempting to add the object is fruitless.

Problem:

When pushing a server_connection object onto a specific VPN_Server struct, it holds this data in the temporary foreach container, but does not apply it.

I Have tried:

adding a custom addConnection() method to the VPN_Server struct

void addConnection(server_connection s_con){
ServerInstancesList.push_back(s_con);
}
Creating a temporary list, adding the server_connection and creating a new VPN_server object, and setting that equal to the current foreach container.

None of these help.

Testing & Indepth description:

In the algorithm, my 1st and 3rd vpn_connection struct have the same IP. On the third iteration, the foreach loop is executed and the following occurs.

VPN_Server ser contains info of the 1st struct info, i.e. an IP and one object in its QList<server_connection> called ServerInstancesList.

ser has an object added, via the addConnection(s_con). Afterwards, the loop is terminated with the return. However ser registered the added object, while outside the foreach loop, no new object was added. Not even to any struct within the list.

It seems to be an easy fix, but I just cannot find it,

help would be appreciated!

Code:

server_connection

struct server_connection{

    //Is a connection contained by IP

    QString cipher,protocol;
    int port;
    bool lzo_compression;

    server_connection(){}

    server_connection(QString _cipher, QString _protocol, int _port, bool _comp){
        cipher = _cipher;
        protocol = _protocol;
        port = _port;
        lzo_compression = _comp;
    }
};

VPN_Server

struct VPN_Server{

    //Holds IP as sort value and list of connection info
    QString VPN_IP;
    QList<server_connection> ServerInstancesList;

    VPN_Server(){
        ServerInstancesList = QList<server_connection>();
    }

    VPN_Server(QString ip, QList<server_connection> server_con_list){
        VPN_IP = ip;
        ServerInstancesList = server_con_list;
    }

    void addConnection(server_connection s_con){
        ServerInstancesList.push_back(s_con);
    }
};

Algorithm:

QList<VPN_Server> data_mananger::parseVPNConnections(QList<VPNConnection> l){

    //Init var
    QList<VPN_Server> server_list = QList<VPN_Server>();
    VPNConnection v_con;
    bool bAdded = false;
    server_connection s_con;

    //processing all vpn_connections, this is raw form sent, contains as fields, ip, cipher, protocol, port, compression
    foreach (v_con, l) {

        //create server_connection - data sorted by ip
        s_con = server_connection(v_con.cipher, v_con.protocol, v_con.port, v_con.lzo_compression);

        //pass through existing data, checking for a matching ip
        foreach (VPN_Server ser, server_list) {
            if (ser.VPN_IP == v_con.ip) {
                //ip match found -> there already exists and ip with a server connection, adding another one with s_con
                ser.addConnection(s_con);
                bAdded = true;
                //break current loop short searching for a matching ip
                break;
            }
        }

        //bAdded = false -> no matching IP has been found, thus creating a nw VPNServer
        if (!bAdded) {
            VPN_Server serv;
            //creating new connection list and adding s_con to this list
            QList<server_connection> list = QList<server_connection>();
            list.push_back(s_con);

            //adding VPNServer to list containing VPNServers
            serv = VPN_Server(v_con.ip, list);
            server_list.push_back(serv);
        }
        else
            //data has been added, reseting add flag
            bAdded = false;

    }
    return server_list;
}

mrjj

Hi

 foreach (VPN_Server ser, server_list)

try with 

 foreach (VPN_Server & ser, server_list)

Else it might be a copy in the loop and u alter a temp. It sounds like such case as it looks fine in debugger but outside the scope ( the foreach) its never happened.

VRonin

even if @mrjj solution is valid I'm afraid it won't work either because:

From: http://doc.qt.io/qt-5/containers.html#the-foreach-keyword

Qt automatically takes a copy of the container when it enters a foreach loop. If you modify the container as you are iterating, that won't affect the loop. [...] using a non-const reference for the variable does not allow you to modify the original container

The same will happen in a range-based for loop.

the solution is using STL iterators:

for (auto serIter= server_list.begin(); serIter!=server_list.end() ;++serIter) {
            if (serIter->VPN_IP == v_con.ip) {
                //ip match found -> there already exists and ip with a server connection, adding another one with s_con
                serIter->addConnection(s_con);
                bAdded = true;
                //break current loop short searching for a matching ip
                break;
            }
        }

CybeX

@mrjj just attempted it, did not work :(

mrjj

@VRonin
Oh thank you. I was not aware of that doc for ForEach :)

mrjj

@CybeX
Sorry my bad, it makes copy as @VRonin informs us.
So ForEach is a no go :)

So use an iterator (preferred) ~~or a good old for loop.~~

VRonin

Bottom line: do not use foreach, for( : ) or java-style itrators unless the container you are iterating is (or can be considered) const

CybeX

@VRonin @mrjj

Thanks for the help guys.

the resultant build errir of:

foreach (VPN_Server & ser, server_list) {
    if (ser.VPN_IP == v_con.ip) {
        ser.addConnection(s_con);
        bAdded = true;
        break;
    }
}

error:

/home/dev/opt/Qt5.7.1/5.7/gcc_64/include/QtCore/qglobal.h:963: error: binding ‘const Misc::VPN_Server’ to reference of type ‘Misc::VPN_Server&’ discards qualifiers
for (variable = *container.i; container.control; container.control = 0)
^

How do you suggest I proceed?

I also tried

I attempted to use a pointer method. I wanted to grab the pointer, and change the values at the memory location, however I get this error:

foreach (VPN_Server *ser, server_list) {
    if (ser->VPN_IP == v_con.ip) {
        ser->addConnection(s_con);
        bAdded = true;
        //break current loop short searching for a matching ip
        break;
    }
}

error:

/home/dev/opt/Qt5.7.1/5.7/gcc_64/include/QtCore/qglobal.h:963: error: cannot convert ‘const Misc::VPN_Server’ to ‘Misc::VPN_Server*’ in initialization
for (variable = *container.i; container.control; container.control = 0)
^

VRonin

Even if you get them to compile they won't work either for the reason mentioned above. use STL iterators instead or the operator[](int) of the list

CybeX

@VRonin a suggest read is a SO post

As mentioned earlier aswel, I will use this iterator (first time :p)

Thanks for all the help!