0

I want to create a vector of pointers that each point to their own Martian object but i can't figure out how to arrange it. I'm currently getting the error

Non-const lvalue reference to type 'vector' cannot bind to a value of unrelated type 'martianDatabase'

but the error constantly changes with every change i make. I've watched a ton of tutorials the last two days trying to figure this out but I'm still stuck.

struct Martian
{
    Martian(string fname, string lname, string ssid);
    string fname, lname, ssid;
    ~Martian();
};


class martianDatabase
{
    vector<Martian*> database;
    martianDatabase();
    void addMartian(vector <Martian*> &database, int &iterator, string f,  string l, string id);
    int iterator = 0;
};

Martian::Martian(string f, string l, string id)
{
    fname = f;
    lname = l;
    ssid = id;
}

void martianDatabase::addMartian(vector <Martian*> &database, int &i, string f, string l, string id)
{
    Martian* m = new Martian(f, l, id);
    database[i].push_back(m);
    i++;
}
M.M
  • 138,810
  • 21
  • 208
  • 365
Ammar
  • 31
  • 7
  • This `Martian::Martian(string f, string l, string id)` would be better as `Martian::Martian(const string& f, const string& l, const string& id)` - Saves copying – Ed Heal May 01 '16 at 01:33
  • Also the use of smart pointers might make your life a bit easier – Ed Heal May 01 '16 at 01:35
  • thank you, changed but the my dilemma remains – Ammar May 01 '16 at 01:37
  • " the error constantly changes with every change i make." - you need to figure out what causes the error, and fix it. Randomly changing the program and hoping to see if the error goes away, isn't a good idea. Even if you do happen to make the error go away, the code might still have a bug – M.M May 01 '16 at 01:44
  • @EdHeal passing by value will be more efficient in some cases – M.M May 01 '16 at 01:45
  • @M.M - In some cases - but not for strings – Ed Heal May 01 '16 at 01:46
  • Really - Having to invoke the copy constructor i.e. allocate memory, copy or a reference? – Ed Heal May 01 '16 at 01:48
  • @EdHeal [see here](http://stackoverflow.com/questions/10231349/are-the-days-of-passing-const-stdstring-as-a-parameter-over) for discussion – M.M May 01 '16 at 01:51

2 Answers2

1
vector<Martian*> database;

Your database is a std::vector of pointers to Martian objects. That's what this declaration states.

database[i].push_back(m);

Since database is a vector, database[i] would be the ith value in this vector. Since this vector is a vector of Martian *, therefore database[i] is some Martian * value in this vector.

Obviously, you understand that if you have Martian *, a pointer to a Martian, it's not a class, and you can't push_back() anything on it. It's a pointer. A plain, garden variety pointer. You can't push_back() anything to a pointer. You can't begin() or end() it, etc...

That's what the compiler is telling you, with it's error message.

And, as far as your "How can i make a vector of pointers" question, you already did it:

vector<Martian *> database;

That's a vector of pointers. Now, whether it's a vector of pointers to dynamically-allocated objects, or not, that's no longer relevant. The vector doesn't care where the objects it's pointing to come from. A pointer to a dynamically-allocated object is exactly the same as a pointer to some object in a static scope.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
1

There are several things wrong with your code:

  • You shouldn't pass your vector<Martian*> as an argument to addMartian, instead you can just access it through the this pointer.
  • There is no need for your int& iterator, as all you are trying to do is add your Martian to the end of a vector
  • Your code database[i].push_back(m); gets the i’th element of the vector database, which is a Martian&, and then trys to call push_back(m) on it, which is not possible, as there is no such function declared for type Martian, what you probably wanted is database.push_back(m), which insert m at the back of the database vector

Consider the following alternative:

class martianDatabase
{
    vector<Martian*> database;
    martianDatabase();
    void addMartian(string f,  string l, string id);
};

void martianDatabase::addMartian(string f, string l, string id)
{
    this->database.push_back(new Martian(f, l, id));
}

Though not realy a problem, it is potentially better to directly initialise member in your constructor (where possible) rather then copy assign them, i.e. use the code:

Martian::Martian(string f, string l, string id) : fname(f), lname(l), ssid(id) { }
Isaac
  • 816
  • 5
  • 12
  • You do not need `this->`. Also smart pointers - in fact why use pointers at all! – Ed Heal May 01 '16 at 01:43
  • @EdHeal, I was only using it to emphasis where the `database` came from, the current object as opposed to a parameter. As to your remark about pointers, if you are referring to `this` being a pointer, I agree it's stupid, it should be a reference. If you are referring to `database` being a vector of pointers, yes it is pointless in this situation, but it is what the OP wanted, I assumed that the OP needed it to be a pointer, for example if he wants to be able to store objects of derived classes in the array, or he want's to later manually delete the Martians without removing from the database. – Isaac May 01 '16 at 01:49
  • @Isaac i just realized that i don't want to push_back, i want to set database[i] = m. Would i still remove my int i in this case? – Ammar May 01 '16 at 02:03
  • @Ammar I am unsure as to why you would want to do that, but in this case you would add an `int i` parameter to the `addMartian` function I gave you, and change the code to `this->database[i] = new Martian(f, l, id);` (MAKE SURE you check that `this->database.size() > i`, If it isn't you probably want to use `push_back`). – Isaac May 01 '16 at 02:07
  • That change gives me No viable overloaded '=' – Ammar May 01 '16 at 02:13
  • @Ammar that's very odd, it should work perfectly, I'm sorry but I have no idead why. (Annother note: `this->database[i] = new Martian(f, I, id)` will replace whatever was previously there, so you may want to delete the original value first, NOTE: I don't know how you plan on using your martianDatabase class so my suggestions may be inapparopriate.) – Isaac May 01 '16 at 02:18
  • @Ammar does your error message give any more details? – Isaac May 01 '16 at 02:18
  • thats all it says. Google says i have to stick const on something because a temporary object cannot bind to a non const reference. I don't know if thats relevant to my code though. – Ammar May 01 '16 at 02:26
  • Are you sure your using my code exactly as I said? Sorry but my compiler is corrently reinstalling, so I can't test it... – Isaac May 01 '16 at 02:29
  • oh, no your code works perfectly fine, i was just trying to implement it into my code. Thanks a ton – Ammar May 01 '16 at 02:33
  • sorry, how would i access an element of the object that the i-th pointer from my database is pointing to? – Ammar May 01 '16 at 02:35
  • Like this: `this->database[i]->lname`, or if you want the entire Martian object, then `*this->database[i]` – Isaac May 01 '16 at 02:46