0

I am working on a basic programme which is designed to dynamically model a road network. The project so far uses a vector of type parent vehicle pointers to store vehicle child objects (cars, vans, lorries etc).

The aim of the project is to calculate and sum the distance travelled by each vehicle child object stored in the parent vector and then reorder the vector based on the distance furthest travelled to distance least travelled.

I am using a sorting algorithm which compares the data member total_dist which stores the summed distance travelled by each object.

Due to an earlier issue, I had to explicitly initialize the total_dist data member to 0 for each object within the vector.

Because of this, the sorting algorithm only "sees" the initialised value (i.e. 0) which means that the vector is not being rearranged.

The natural solution to this (at least in the mind of a novice programmer like myself) is to alter the the function which calculates total_dist to accept a pass by reference in order to update the data member as the programme runs.

However, I'm not actually sure if this is syntactically possible. This is what I currently have (I know my code is probably really ugly and inelegant but I'm still very new to programming in general).

Method

    void Vehicle::set_total_dist(float &f) {
    total_dist += f;           //total_dist is public Vehicle data member
    }

I know setting data members to public is undesirable but It's the only way I get get my algorithm to work within my current abilities.

Vector Initialisation

    vector<Vehicle*> vehicle_lanes; // 'Vehicle' is the parent class        

    // Populating vector with respective child vehicle objects
    vehicle_lanes.push_back(new Car);       
    vehicle_lanes.push_back(new Van);
    vehicle_lanes.push_back(new Bus);
    vehicle_lanes.push_back(new Lorry);
    vehicle_lanes.push_back(new Motorbike);

    // Setting child object names for identification
    vehicle_lanes[0]->set_name("Car");
    vehicle_lanes[1]->set_name("Van");      
    vehicle_lanes[2]->set_name("Bus");
    vehicle_lanes[3]->set_name("Lorry");
    vehicle_lanes[4]->set_name("Motorbike");

    // Initialising total distance travelled by each object to 0 meters
    vehicle_lanes[0]->total_dist = 0;       
    vehicle_lanes[1]->total_dist = 0;       
    vehicle_lanes[2]->total_dist = 0;
    vehicle_lanes[3]->total_dist = 0;
    vehicle_lanes[4]->total_dist = 0;

If total_dist isn't explicitly initialised, the initial summed value isn't defined therefore set_total_dist() calculates what i believe is to be some sort of alphanumeric error value.

Vector Iterator and Sorting Algorithm

    for (vector<Vehicle*>::iterator it = vehicle_lanes.begin(); it != 
    vehicle_lanes.end(); it++) {
        
        // Vehicle sort function
        sort(vehicle_lanes.begin(), vehicle_lanes.end(), 
        CompareVehicleLocation);

        // This is the problem// 
        (*it)->set_total_dist((*it)->get_grid_location());

        // Printing the object name and it's associated Total_dist
        cout << (*it)->get_name() << "'s total distance travelled is: " << 
        (*it)->get_total_dist() << " units." << endl;
    }

The compiler doesn't like when I'm passing (*it)->get_grid_location() by reference but compiles just fine when I'm passing the argument by value. When trying to pass be reference, the IDE shows a red underline between the double left-hand parentheses ((.

The error i get before compilation, when trying to pass by reference is:

initial value of reference to non-const must be an lvalue

After compilation, I get:

Error C2664 'void Vehicle::set_total_dist(float &)': cannot convert argument 1 from 'float' to 'float &'

I have tried searching for posts identical to mine but I haven't had any luck, the issues I found which were somewhat related to mine such as What's wrong with passing C++ iterator by reference? . Didn't make much sense to me due to my inexperience.

Any advice would be greatly appreciated. I'll be happy to supply any additional, relevant information I can. Thank you.

Community
  • 1
  • 1
  • 1
    There is no sense in passing it by reference, you don't modify it in the function. – HolyBlackCat Apr 08 '18 at 11:40
  • As for the error, I *guess* that `get_grid_location()` returns a `float` *by value*? – Some programmer dude Apr 08 '18 at 11:41
  • @HolyBlackCat My understanding was that passing by reference does permanently modify it in the function. Which is what I'm aiming to do. Are there any alternatives you could suggest if my current approach doesn't make sense? – Squire1998 Apr 08 '18 at 11:42
  • @Someprogrammerdude yes, that is correct – Squire1998 Apr 08 '18 at 11:43
  • Yes, you would need a reference if you modified `f`. But you don't. – HolyBlackCat Apr 08 '18 at 11:49
  • @HolyBlackCat ahh, okay I see. I'm obviously thinking about this all wrong. Thanks. Regardless, if you were to just humour my ignorance, how would I syntactically go about passing an iterator by reference to a function? I think I'll need to do this eventually so I't be handy to know. – Squire1998 Apr 08 '18 at 11:54
  • As with any other type: `void foo(vector::iterator &blah) {...}` – HolyBlackCat Apr 08 '18 at 12:11

2 Answers2

0

Quick Answer: A left-value is something you can modify. Something that occupies a certain amount of memory. A right-value is something temporary, e.g. instant number, normal return value of a function, etc.

Your error cannot convert from 'float' to 'float &' is to pass a right-value float to a function which takes a left-value reference (float&). That means your function will presumably change the parameter it takes. But right-value has no address, hence cannot be changed. That's why the compiler is refusing your codes.

Another glitch is that the sort function in your code should be outside the for loop.

scinart
  • 457
  • 2
  • 9
0

I found a couple of issues in your program:

1) Some simplification is required. You need not set the vehicle name if it is integrated with the constructor.

2) The total_distance member variable could be initialize to zero in the class definition itself.

3)You were trying to sort the vehicles before the total_distance for each vehicle was set.

4) Use auto keyword to replace long type-names.

Try this ( I am using a dummy distance generator as your grid logic is not so clear):

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


using namespace std;


class Vehicle
{
protected:
    string name;
    int total_dist{0};   // distance will be set to 0 on any new vehicle
public:
    Vehicle() {}
    ~Vehicle() {}
    virtual string get_name() = 0;
    int get_total_dist() const { return total_dist; }
    void set_grid_location(int d) {total_dist = d;}
};

class Car :public Vehicle 
{
public:
    Car() 
    {
        name = "Car";  //name will be automatically set
    }

    string get_name() { return name; }
};

class Bus :public Vehicle
{
public:
    Bus()
    {
        name = "Bus";
    }

    string get_name() { return name; }
};

int location_generator() // testing only - use your own method
{
    int arr[] = {4,2,1,5,6,2,3 };
    static int count = 0;

    return arr[count++ % 6];
}

int main()
{
    vector<Vehicle*> vehicle_lanes;       

    //create some vehicles
    vehicle_lanes.push_back(new Car);
    vehicle_lanes.push_back(new Bus);

    //filling the location of vehicles before sorting
    for (auto& it = vehicle_lanes.begin(); it != vehicle_lanes.end(); it++)  
    {
        (*it)->set_grid_location(location_generator());
    }

    //sorting
    auto CompareVehicleLocation = [](const Vehicle* lhs, const Vehicle* rhs)->bool { return lhs->get_total_dist() < rhs->get_total_dist(); };

    sort(vehicle_lanes.begin(), vehicle_lanes.end(), CompareVehicleLocation);


    //printing sorted vector
    for (auto& it = vehicle_lanes.begin(); it != vehicle_lanes.end(); it++)
    {
        cout << (*it)->get_name().c_str() << "'s total distance travelled is: " <<
            (*it)->get_total_dist() << " units." << endl;
    }

    //delete the pointers to avoid memory leak at the end
    for (auto& it = vehicle_lanes.begin(); it != vehicle_lanes.end(); it++)
    {
       delete (*it);
    }
    return 0;
}
seccpur
  • 4,996
  • 2
  • 13
  • 21