5

I have an class A:

class A {
  int value1;
  int value2;
  std::string text;

public:
  A(int value1, int value2, std::string text) 
   : value1(value1), value2(value2), text(text) { }
};

And some "container" class B:

class B {
  std::vector<A> objects;
  ...

public:
  ...
  void addObject(A a) {
    objects.push_back(a);
  }
};

And code:

B b;
A a(2, 5, "test");
b.addObject(a);
//I no longer need a from now on

My problem is, how to optimize B::addObject(A a) to avoid any copying. What I want to achive is to add new object of type A to B.objects via B's method.

PolGraphic
  • 3,233
  • 11
  • 51
  • 108

2 Answers2

6

There are many answers to this question. You could just change your B function to move(), and move() into that function:

void addObject(A a) { objects.push_back(std::move(a)); }

b.addObject(std::move(a)); // two moves

You could add overloads of addObject that take by const lvalue reference and by rvalue reference

void addObject(A const& a) { objects.push_back(a); }
void addObject(A&& a) { objects.push_back(std::move(a)); }

b.addObject(std::move(a)); // one move

You could add a function template for addObject that emplaces:

template <class... Args>
void addObject(Args&&... args) { objects.emplace_back(std::forward<Args>(args)...); }

b.addObject(2, 5, "test"); // zero moves

Either way, there's a completely unnecessary copy here:

A(int value1, int value2, std::string text) 
: value1(value1), value2(value2), text(text) { }

you want:

A(int value1, int value2, std::string text) 
: value1(value1), value2(value2), text(std::move(text)) { }
Barry
  • 286,269
  • 29
  • 621
  • 977
  • Thank you :) One question: is the `text(std::move(text))` from your side note completely safe in all cases? Also, isn't it something that the compiler would do automatically either way (move instead of copy there because of optimization)? – PolGraphic Sep 27 '17 at 16:05
  • 1
    @PolGraphic Yes - the constructor owns `text` so it's safe to move from. And no, it won't. – Barry Sep 27 '17 at 16:34
  • thanks for the further explanation. It's a bit around the topic but does `A a(2,5,"test"); b.addObject(std::move(a));` in second solution is any different from`b.addObject(A{2,5,"test"});` in case of performance (the second one creates `A` and than moves it or not really)? – PolGraphic Sep 27 '17 at 18:07
  • @PolGraphic The moved-from `A` will destroyed within `addObject()` in the second case, but at the end of the scope of `a` in the first case. But, basically the same. – Barry Sep 27 '17 at 18:32
1

You might change the parameter type of addObject and use emplace_back, to avoid constructing an object of A which won't be used later and copying it into the vector. e.g.

void addObject(int value1, int value2, std::string text) {
  objects.emplace_back(value1, value2, text);
}

and

B b;
b.addObject(2, 5, "test"); // construct the element directly into the vector without constructing A
songyuanyao
  • 169,198
  • 16
  • 310
  • 405
  • That's an good one, but there is one small problem for me with it. For now I don't relate on `A` constructor parameters anywhere in `B` header file. And with your solution it would require to give more information (that I think is more related to `A` than `B`) in `B` header file. It's more the matter of the appearance tho. – PolGraphic Sep 27 '17 at 16:07
  • @PolGraphic I didn't get what you mean, you meant adding `#include "A.h"` in `B.h`? No, it should be same with the one using `push_back`, if the member fucntion is defined in `B.cpp`, it's not needed. – songyuanyao Sep 27 '17 at 16:11
  • No, I mean that inside of `B.h` I will have to know & remember about `A` constructor parameters order. And in case of changes with it, I will also have to modify `B.h`. – PolGraphic Sep 27 '17 at 16:16