Iterating using two range-for loops

[longberns]

Hi,
I am yet to figure out what is wrong with my program.

I have a stations, trains and vehicles(wagons). Train receives wagon from station. If received, station deletes that wagon so that another train will not have it again. This is what i have put so far. Comments are there to explain what i am doing

void Simulator::makeStatistics(vector<Train>& aTrains, vector<Train>& iTrains)
{
vPtr vp; // shared_ptr<Vehicle> vp;
vector<VehicleType> type;

for (auto &trn : trains) // for a train in the vector of trains
{
	for (auto &stn : stations) // for a station in the vector of stations
	{
		if (trn.getDepStation() == stn.getStationName()) // if trainDeparture is station
		{
			type = trn.getTrainVehTypes(); // get the types of wagons needed for train

			for (auto it = type.begin(); it != type.end();it++) // for every wagon
			{
				if (stn.requestVehicle(*it, vp)) // if this wagon is found in the station, return a wagon
				{
					trn.addVehicle(vp);			// add this wagon to the train
					stn.removeVehicle(vp->getVehid()); // remove this wagon from the station
				}
			}
			if (trn.hasAllVehicles()) // if train has all its wagons
			{
				trn.setTrainStatus(ASSEMBLED); // change train state to ASSEMBLED
				aTrains.push_back(trn); // save to assembled train vector
			}
			else
			{
				trn.setTrainStatus(INCOMPLETE); // otherwise, train is INCOMPLETE
				iTrains.push_back(trn); // save to incomplete train vector
			}
		}
	}
}
}
//===================
// THE OTHER CODES
/// ===================
void Train::addVehicle(vPtr & wagon)
{
	vehiclesNeeded.push_back(wagon);
}

///
void Station::removeVehicle(int id)
{
	auto it = find_if(vehicles.begin(), vehicles.end(), [&id](const vPtr &vp) {return vp->getVehid() == id; });
	if (it != vehicles.end())
	{
		vehicles.erase(it);
	}
}

///
bool Station::requestVehicle(VehicleType type, vPtr &veh)
{
	auto it = find_if(vehicles.begin(), vehicles.end(), [&type](const vPtr &vp) {return vp->getVehType() == type; });
	if (it != vehicles.end())
	{
		veh = *it;  // return the found vehicle
		return true;
	}
	return false;
}

The problem is that, it seems as if more vehicles than necessary are deleted from the station such that i have more trains as INCOMPLETE than expected.

Is it my way of looping? Could this be done in a different way?

[JLBorges]

We can't erase elements in the middle of a range-based loop; erase invalidates iterators.
Instead, use a classical for-loop, and use the iterator returned by erase to continue the iteration.
Or (better) use the erase-remove idiom. https://en.wikipedia.org/wiki/Erase-remove_idiom

#include <iostream>
#include <vector>
#include <algorithm>

int main()
{
    {
        std::vector<int> vec { 0, 1, 2, 3, 4, 5, 6, 7,8, 9 } ;

        for( auto iter = vec.begin() ; iter != vec.end() ; )
        {
            // erase returns a valid iterator to the element immediately after the
            // one that was erased, or end() if the last element was erased .
            if( *iter % 2 ) iter = vec.erase(iter) ;
            else ++iter ; // not erase, increment the iterator
        }

        for( int v : vec ) std::cout << v << ' ' ;
        std::cout << '\n' ;
    }

    {
        std::vector<int> vec { 0, 1, 2, 3, 4, 5, 6, 7,8, 9 } ;
        
        // erase-remove idiom
        const auto is_odd = []( int v ) { return v%2 == 1 ; } ;
        vec.erase( std::remove_if( vec.begin(), vec.end(), is_odd ), vec.end() ) ;

        for( int v : vec ) std::cout << v << ' ' ;
        std::cout << '\n' ;
    }
}

http://coliru.stacked-crooked.com/a/f2a3d481697ea1f4

[longberns]

@JLBorges
This would have worked so well if i were to delete v from vtype.

I am actually not deleting v but the vehicle returned (as a type of v). That is what this function
does. It takes the type of vehicle, searches it and returns the vehicle.

Is it possible(maybe) to both return and delete on the same function then?

bool Station::requestVehicle(VehicleType type, vPtr &veh)
{
	auto it = find_if(vehicles.begin(), vehicles.end(), [&type](const vPtr &vp) {return vp->getVehType() == type; });
	if (it != vehicles.end())
	{
		veh = *it;  // return the found vehicle
		return true;
	}
	return false;
}

for (auto &v : vtype) // for every wagon
{
	if (stn.requestVehicle(v, vp)) // if this wagon is found in the station, return a wagon
	{
		trn.addVehicle(vp);			// add this wagon to the train
		stn.removeVehicle(vp->getVehid()); // remove this wagon from the station

		if (trn.hasAllVehicles()) //if types.size() == vehiclesNeeded.size()
		{
			trn.setTrainStatus(ASSEMBLED); // change train state to ASSEMBLED
			aTrains.push_back(trn); // save to assembled train vector
		}
		else
		{
			trn.setTrainStatus(INCOMPLETE); // otherwise, train is INCOMPLETE
			iTrains.push_back(trn); // save to incomplete train vector
		}
	}
}

[Enoizat]

Could you please provide a compilable code which reproduces your issue?