more pointers

Started by
20 comments, last by pbivens67 1 year ago

Well, I figured out how to print out the person vector all I want to do is populate the car vector and print it out as well. Can you please keep the commentary to a minimum. All I need is an answer to my question.

class Person
{
public:
	Person(int p_age,string p_name)
	{
		age = p_age+4;
		name = p_name;
	}
	int age;
	string name;
private:
};

class Car
{
public:
	Car(string p_model, Person* p_owner, Person* p_driver) 
	{
		model = p_model;
		owner = p_owner;
		driver = p_driver;
	}
	string model;
	Person* owner;
	Person* driver;
private:

};

int main()
{
	vector<Person*> person = { new Person(55,"Phil"), new Person(61,"Sylvia"), new Person(65,"Jeff") };
	vector<Car*> car = {new Car("F-100"),new Person(57,"Phil"),new Person(59,"Jeff")};
	for (vector<Person*>::iterator it = person.begin(); it != person.end(); it++)
	{
	cout << (*it)->age << " " << (*it)->name << endl;
	}

Advertisement

Why are you adding People to the car vector?

There's also some syntactic sugar when it comes to iterators that makes the for loop a bit more concise and easier to read:

for (const Person* p : person) {
	cout << p->age << " " << p->name << "\n";
}

quou said:
Why are you adding People to the car vector?

I want to add the owner and driver to the car vector, the owner and driver consists of age and name. this is what the exercise calls for.

You need to pass the owner and driver People into the car's constructor (within the parenthesis where you pass the model). It's not possible to add them to the vector directly because their type is not compatible with Car*.

Some other things that can improve:

  • Pass string by const reference. i.e. rather than parameter type ‘string’, use ‘const string&’. This is faster for complex objects (i.e. not primitive types) because it reduces the number of copies/moves.
  • Every time you use the ‘new’ operator, you are leaking memory unless you later call delete on the pointer that was returned by new. It would be better use unique_ptr (if the pointer implies ownership), or to not use new at all and allocate objects on stack, or inside a vector (vector<Car> instead of vector<Car*>). In C++ it's important to think about who owns what, and to ensure that things get cleaned up when they are no longer needed. Avoid the use of shared_ptr unless ownership is actually shared.

Aressera said:
You need to pass the owner and driver People into the car's constructor (within the parenthesis where you pass the model). It's not possible to add them to the vector directly because their type is not compatible with Car*.

how do you pass the owner and driver to the car's constructor, I thought that it was coded correctly.

Your Car constructor takes a string and two pointers. When you create a car (with new in your case), you pass through those arguments. You're passing a string to your Car, but not the pointers to people.

If you were to compile this code the errors you get are pretty descriptive of what's gone wrong.

Here's a version of your code using unique_ptr. I added messages when something is constructed or destructed. I recommend you do this while you are learning this part of the language. If you do this in your earlier attempts, you'll see that your objects are not being destructed, and that's a problem.

#include <iostream>
#include <vector>
#include <string>
#include <memory>

struct Person {
  Person(int p_age, std::string p_name)
    : age(p_age + 4), name(p_name) {
    std::cout << "Person constructed" << std::endl;
  }
  
  ~Person() {
    std::cout << "Person destructed" << std::endl;
  }
  
  int age;
  std::string name;
};

std::ostream &operator<<(std::ostream &os, Person const &person) {
  return os << "(age: " << person.age << ", name: " << person.name << ")";
}

struct Car {
  Car(std::string p_model, std::unique_ptr<Person> p_owner, std::unique_ptr<Person> p_driver)
    : model(p_model), owner(std::move(p_owner)), driver(std::move(p_driver)) {
    std::cout << "Car constructed" << std::endl;
  }

  ~Car() {
    std::cout << "Car destructed" << std::endl;
  }
  
  std::string model;
  std::unique_ptr<Person> owner;
  std::unique_ptr<Person> driver;
};

std::ostream &operator<<(std::ostream &os, Car const &car) {
  return os << "(model: " << car.model << ", owner: " << *car.owner << ", driver: " << *car.driver << ")";
}

int main() {
  std::vector<std::unique_ptr<Person>> people;
  people.push_back(std::make_unique<Person>(55,"Phil"));
  people.push_back(std::make_unique<Person>(61,"Sylvia"));
  people.push_back(std::make_unique<Person>(65,"Jeff"));

  std::vector<std::unique_ptr<Car>> cars;
  cars.push_back(std::make_unique<Car>("F-100", std::make_unique<Person>(57,"Phil"), std::make_unique<Person>(59,"Jeff")));
  
  for (auto const &p : people)
    std::cout << *p << std::endl;
  
  for (auto const &c : cars)
    std::cout << *c << std::endl;
}

I'll stay out of this one from here on. I think my points are being made on all sides without me anyway.

I found out that if you put the first post as a question to GPT-4, it writes precisely what the OP wanted. You can then ask about what's wrong with the code and ways to improve it, and it tells you a lot of the things others have been saying in this and the previous thread.

pbivens67 said:
Car(string p_model, Person* p_owner, Person* p_driver)

This is your Car constructor, it takes a string and 2 Person objects

pbivens67 said:
new Car("F-100")

Here you try to make a Car object using the constructor. However you “new” call only has a string, it misses the 2 Person objects.

pbivens67 said:
,new Person(57,"Phil"),new Person(59,"Jeff")

This text creates 2 Person objects. It must be inside the parentheses of “new Car” call. In your code that text is after the closing parenthesis of the “new Car” call.

This topic is closed to new replies.

Advertisement