What is the best way to access elements of a class member container from outside that class in c++?

Question

In the following example, using pointers has caused me some complications that I'd like to avoid. So I'm wondering that the correct way of dealing with this situation would be.

So I have a struct called Cars. It's a container to hold objects of class Car. Specifically, The Cars class has a member variable that stores all of the Car objects in a vector. An instantiation of Cars exists in the scope of a simulation that needs to access a variable number of Car objects within Cars.

To describe my issue here is an example sim of cars honking. While I know the following example can be improved in many obvious ways to better simulate cars honking, keep in mind this example's purpose is to show the structure of a much more complicated simulation whose structure I cannot broadly alter very too much.

struct Car {
   public:
       std::string id_;
       std::string make_;
       Car(const std::string& id, const std::string make) : id_(id) make_(make) {}
       void honk(){
          std::cout<<"Car "<<id_<<" honked!"<<std::endl;
       };
};

class Cars {
private:
    std::vector<Car> garage;

public:
    void addCar(const Car& car) {
        cars.push_back(car);
    }

    // Returns reference to element in garage vector
    Car& get_car_by_id(const std::string id){
        for (Car& car: garage) {
            if (car.id_ == id) return car;
        }
    }

    // Returns a vector of pointers to elements in garage vector
    vector<std::unique_ptr<Car>> get_cars_by_make(const std::string &make) {
        vector<std::unique_ptr<Car>> subset;
        for (Car& car: garage) {
            if (car.make == make) {
                subset.push_back(std::make_unique<Car>(car));
            }
        }
        return subset;
    }
};

In the following small simulation we use the two different getter methods to access Car objects: via a single reference to access an individual object, and then a vector of smart pointers to access a variable number of objects based on some condition that can change throughout the simulation. The latter is the crux of my issue.

int main() {
    Cars fleet;

    fleet.addCar(Car("1", "Toyota"));
    fleet.addCar(Car("2", "Toyota"));
    fleet.addCar(Car("3", "Tesla"));

    // Honk horn for car 3
    Car& car fleet.get_car_by_id("3");
    car.honk();

    // Honk all of the Toyota's horns
    vector<std::unique_ptr<Car>> cars = garage.get_by_make("Toyota");
    for (const auto car : cars) {
        car.honk();
    }
}
   

My question is this: Is there a better way to provide access to these Car objects? I've been trying to avoid pointers as much as possible but I feel like they're the only thing that makes sense. Short of providing the index values of the objects instead of pointers (which sounds disgusting), I'm not sure what to do.

Answer 1

get_cars_by_make() does not return a vector of pointers to cars in the garage. It returns a vector of unique pointers to new cars that were copy-constructed from the cars in the garage.

If you want to return pointers to the cars already in the garage, return vector<Car*> instead of vector<unique_ptr<Car>>.

Edit: I realize that this is a minimal example, but if get_cars_by_make() doesn't do anything but a linear search, the method doesn't do anything a client couldn't do for itself if it could see the vector iterators. If you exported those iterators (ideally as a range, but possibly with begin/end methods or even returning a reference to the vector), then the client could find by make without having to allocate a vector to hold the results.

Answer 2

Building off of user1806566's answer, I think there's some confusion around the difference between a vector of unique_ptr's and a vector of * pointers. This is something I struggled to understand early on. 'unique_ptr' is allocating memory for new objects on the heap, which in many cases like this is unnecessary. A pointer using * but without any sort of smart pointer or 'new' keyword acts as a reference to an existing object on the stack. This is the more simple approach, and avoids unnecessary complication that often comes with using the heap. I recommend doing some reading on the differences between the stack and the heap.

Answer 3

I would make Cars simply a database of shared_ptr using std::map set std::set to manage the collections (or the unordered version). That way, we can keep the same car in different collection tables (id to car, make to list of cars, etc...). Then we don't make redundant copies of a Car. And we can share the pointer to Car rather freely without worry of a dangling pointer if the "fleet" destructs while someone else is driving the car.

The use of auto keyword in this example is not lost on me. :)

class Cars {

private:
    std::vector<std::shared_ptr<Car>> garage;
    std::unordered_map<std::string, std::shared_ptr<Car>> idToCarTable;
    std::unordered_map<std::string, std::unordered_set<std::shared_ptr<Car>>> makeToCarsTable;

public:
    void addCar(const Car& car) {
        auto spCar = std::make_shared<Car>(car);
        garage.push_back(spCar);
        idToCarTable[spCar->id_] = spCar;
        makeToCarsTable[car.make_].insert(spCar);
    }

    std::shared_ptr<Car> get_car_by_id(const std::string& id) {
        auto itor = idToCarTable.find(id);
        if (itor != idToCarTable.end()) {
            return itor->second;
        }
        return std::shared_ptr<Car>(); // null
    }

    std::unordered_set<std::shared_ptr<Car>> get_cars_by_make(const std::string& make) {
        std::unordered_set<std::shared_ptr<Car>> resultSet;
        auto itor = makeToCarsTable.find(make);
        if (itor != makeToCarsTable.end()) {
            // make a copy of the set
            resultSet = itor->second;
        }
        return resultSet;
    }
};

Then your main is mostly what you have:

int main() {
    Cars fleet;

    fleet.addCar(Car("1", "Toyota"));
    fleet.addCar(Car("2", "Toyota"));
    fleet.addCar(Car("3", "Tesla"));

    // Honk horn for car 3
    auto car3 = fleet.get_car_by_id("3");
    if (car3 != nullptr) {
        car3->honk();
    }

    // Honk all of the Toyota's horns
    auto toyotas = fleet.get_cars_by_make("Toyota");
    for (const auto& toyota : toyotas) {
        toyota->honk();
    }
}