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++;
}
Improve this question
8
  • 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 Commented May 1, 2016 at 1:33
  • Also the use of smart pointers might make your life a bit easier Commented May 1, 2016 at 1:35
  • thank you, changed but the my dilemma remains Commented May 1, 2016 at 1: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 Commented May 1, 2016 at 1:44
  • @EdHeal passing by value will be more efficient in some cases Commented May 1, 2016 at 1:45

2 Answers 2

Reset to default
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) { }
Improve this answer
Sign up to request clarification or add additional context in comments.

12 Comments

You do not need this->. Also smart pointers - in fact why use pointers at all!
@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 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 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).
That change gives me No viable overloaded '='
|
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.

Improve this answer

2 Comments

Ah so it would be database[i] = m;
That would be valid C++ code, that should compile without any issues. Whether this will do what you really want it to do, is a completely separate question.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.