c++ classes and vector members

Question

I have checked Stack Overflow for a sample of these class vectors and none of the answers point out common uses of vectors in a class . My code works but I have a few questions on it .

I have a struck of objects that is stored in a class member vector

struct Station
{
    std::string StationName;
    int StationId;
    int PlayerId;
    std::vector<float> position;

};

Here is the class that declaration with the Vectors

class playerData
{
public:
    std::vector <playerDataDetails> PlayerList;
    std::vector<Station> Stations;
    void addNewPlayer(std::string Name , std::string faction , int Id );
    void PrintPlayer();
    void AddStations(int PlayerNumber, std::string name);

};

When running the member function this is how Im accessing the Vector

 void playerData::AddStations(int PlayerNumber, std::string name)
{
    // p[i].
     Station s ;
     s.StationName = name;
     s.StationId = 1; 
     s.PlayerId = PlayerNumber;
     s.position = { 2.1f, 1.1f , 1.1f};

    // s.position = { { 2.1f }, { 2.3f }, { 2.3f } };

    

    this->Stations.push_back(s);

 }

My question is this .

this->Stations.push_back(s);

Is this the correct way to add station to the stations vector ?
Is this copy or move Semantics ?
Is there another way of doing this with pointers and references ?
And is this the most efficient way of working with Vectors in a class ?

100% was testing all things out using it including a static call ... Removed it ... — JonoJames, Jul 11 '22 at 13:37
Yes, Copy (but you could also emplace it directly), Yes (std::shared_ptr, unique_ptr, or whatever), opinion-based; For me the biggest issue with your examples is the naming convention. Name `YourClasses`, `yourVariables`, and `_yourClassMermbers` in some unified way. — pptaszni, Jul 11 '22 at 13:40
I'd assume one-lining it as: `this->Stations.emplace_back(std::move(name), 1, PlayerNumber, { 2.1f, 1.1f , 1.1f})` would be the more efficient solution. Might need to explicitly make the final argument a `vector`, e.g. `this->Stations.emplace_back(std::move(name), 1, PlayerNumber, std::vector{ 2.1f, 1.1f , 1.1f})`; I'm not confident it will convert the initializer list to a `vector` for you through the `emplace_back` (but it might, *shrugs*). The one-liner would only construct a single `Station` with no moves of `Station`, where your existing code makes two `Station`s. — ShadowRanger, Jul 11 '22 at 13:40
Yes it's correct, it's copy semantics, to possibly use move semantics replace `push_back` with `emplace_back`. Most efficient is highly subjective. When I was a professional developer most efficient meant least likely to be buggy, most likely to be understood by a future maintenance developer, most robust to future changes etc. Important stuff like that. — john, Jul 11 '22 at 13:40
The minimalist improvement would be just to change `this->Stations.push_back(s);` to `this->Stations.push_back(std::move(s));` which would still have two instances of `Station`, but would at least use move semantics so the `vector` would only exist once. One-lining with `emplace_back` might avoid even that as I noted, but it does lead to more dense code. — ShadowRanger, Jul 11 '22 at 13:43
`s.StationName = name;` -- There are multiple (unnecessary) copies of the string going on. The parameter being passed is by value (incurs a copy) here: `void playerData::AddStations(int PlayerNumber, std::string name)`, and then the assignment shown above. Pass the string by const reference, not by value. — PaulMcKenzie, Jul 11 '22 at 13:45
@JonoJames -- Also note that a `struct` can have constructors, destructors, virtual functions, etc. just like a `class`. Thus `Station` should have a constructor that takes a player number and name. — PaulMcKenzie, Jul 11 '22 at 13:49
@PaulMcKenzie: Receiving arguments you intend to keep a copy of by value is reasonable (avoids risk of exceptions within your function, potentially optimizes when the value passed is an r-value by constructing directly into the argument and only moving, where receiving by const reference means constructing, then copy constructing internally at least once). They should be `std::move`-ed from when making the local copy though. — ShadowRanger, Jul 11 '22 at 13:51
@ShadowRanger Yeah, I was going to amend the comment and add that it could be std::moved. — PaulMcKenzie, Jul 11 '22 at 13:53
As an alternative, you could `Station & s = Stations.emplace_back();`, which would default construct a station that you then fill in the members of. (`Stations.emplace_back(); Station & s = Stations.back();` prior to C++17) — Caleth, Jul 11 '22 at 13:54
Just checked. The legal syntax for the one-liner is either `this->Stations.push_back({std::move(name), 1, PlayerNumber, {2.1f, 1.1f , 1.1f}});` or `this->Stations.emplace_back(Station{std::move(name), 1, PlayerNumber, {2.1f, 1.1f , 1.1f}});`. `this->Stations.emplace_back(std::move(name), 1, PlayerNumber, std::vector{2.1f, 1.1f , 1.1f});` with individual arguments only works if you explicitly define a constructor for `Station`. — ShadowRanger, Jul 11 '22 at 13:59
Are you sure about using `std::vector`? Because this is not the kind of vector used in physics. When dimensionality of your space is constant then `float[3]` is much more efficient. `vector` - that makes sense. Also, in games the time of adding is usually irrelevant (when it happens once), more important is access time (the one that happens once per tick, desirably 60 times per second) — Agent_L, Jul 11 '22 at 15:37

ShadowRanger · Accepted Answer · 2022-07-11T14:14:19.530

Is this the correct way to add station to the stations vector ?

It's a perfectly fine approach, just a little inefficient.

Is this copy or move Semantics ?

Copy semantics (this->Stations.push_back(s); invokes the version of push_back taking a const reference and copies it into the vector).

Minimalist changes could avoid unnecessary copies by changing s.StationName = name; to s.StationName = std::move(name); and this->Stations.push_back(s); to this->Stations.push_back(std::move(s));, which is enough to replace all copies with moves (though additional changes could reduce the number of moves and potentially eliminate some temporaries).

Is there another way of doing this with pointers and references ?

Yes. Pointers don't help, but more careful use of move semantics and/or emplace* semantics (to avoid temporaries entirely) could improve things (see below).

And is this the most efficient way of working with Vectors in a class ?

No. This could be done far more efficiently by avoiding some unnecessary copies and/or moves.

Without changing Station, you could replace the entire body of AddStations with either:

this->Stations.push_back({std::move(name), 1, PlayerNumber, {2.1f, 1.1f , 1.1f}});

or:

this->Stations.emplace_back(Station{std::move(name), 1, PlayerNumber, {2.1f, 1.1f , 1.1f}});

adding std::move() around name to avoid a copy in favor of a move, and avoiding a copy of Station (it probably still needs to move-construct it into the vector though).

If you defined an explicit constructor for Station, e.g. Station(std::string sn, int sid, int pid, std::vector<float> pos) : StationName{std::move(sn)}, StationId(sid), PlayerId(pid), position(std::move(pos)) {}, you could also use:

this->Stations.emplace_back(std::move(name), 1, PlayerNumber, std::vector<float>{2.1f, 1.1f , 1.1f});

which would definitely avoid a move of a Station object itself (though it still has to move the name and the temporary vector in the process of constructing it directly into the Stations).

The various forms of weirdness in why each form of push_back or emplace_back works, or doesn't, with varying levels of explicitness is explained on this question.

Great answer I liked the idea of using the explicit constructor with an initialization list as its far cleaner . I will serialize the data into a binary stream so what I was hoping to desterilize it in the following way . std::vector Stations[4] = { { "A10", 1, 1, std::vector{2.1f, 1.1f , 1.1f}, { "A11", 1, 1, std::vector{2.1f, 1.1f , 1.1f}, { "A12", 1, 1, std::vector{2.1f, 1.1f , 1.1f}, { "A13", 1, 2, std::vector{2.1f, 1.1f , 1.1f} }; but I work around how to do it affectively — JonoJames, Jul 11 '22 at 18:36

c++ classes and vector members

1 Answers1