24

I am currently going through the book Head First Object Oriented Analysis and Design and also getting used to some of the features of C++11 (especially unique_ptr and move semantics) at the same time. In the book, they give the design of a strategy board game as an example. It has a board, tiles and units on tiles. The code is in Java and I was trying to rewrite it in C++. In Java, the tile class is as follows:

public class Tile
{
    private List units;

    public Tile()
    {
        units = new LinkedList();
    }

    protected void addUnit(Unit unit)
    {
        units.add(unit);
    }

    protected List getUnits()
    {
        return units;
    }
}

I did the same thing in c++ first using shared_ptr. It worked but very slow compared to my second version which used raw pointers. I then tried unique_ptr:

class Tile
{

public:
  Tile();
  virtual ~Tile();

  void addUnit()
  {
    mUnits.push_back(std::unique_ptr<Unit>(new Unit());
  }

  std::vector<std::unique_ptr<Unit>> getUnits()
  {
    return mUnits;
  }

private:
  std::vector<std::unique_ptr<Unit>> mUnits;

};

the compiler does not like the getUnits. From what I understand it comes from the fact that the copy constructor is disabled in the unique_ptr. How would you return the list of vectors? Thanks!

Xeo
  • 129,499
  • 52
  • 291
  • 397
darkman
  • 343
  • 1
  • 2
  • 5
  • 2
    Return it by reference: `std::vector>& getUnits()` – hmjd Sep 06 '12 at 14:50
  • 3
    Returning a vector of unique_ptrs would mean giving them away, so that the Tile no longer has access to them. Is that what you want? – Vaughn Cato Sep 06 '12 at 14:51
  • 4
    Do you really need to return the vector? If your class wraps `std::vector`, you should provide better access to its data than this accessor. This example clearly states that no-brainy rewrite from Java to C++ **isn't** the way to go. Especially because you don't provide any new functionality. – Bartek Banachewicz Sep 06 '12 at 14:52
  • 2
    Don't forget, Java "objects" are pointers; even with what looks like a return by value, the result points to the original object. The analog in C++ is to return a pointer or reference. – Pete Becker Sep 06 '12 at 14:59
  • If you really wanted to *remove* the vector, you could say: `std::vector> res; res.swap(mUnits); return res;` But that would empty out the `mUnits` vector. – Kerrek SB Sep 06 '12 at 15:17
  • 1
    The original class is poorly designed anyway, hopefully that book mentions that. Why encapsulate a container just to give complete access to it? – GManNickG Sep 06 '12 at 18:38
  • Why do you need pointers in the first place? Just store a `vector` of `Unit` and return references to them if a caller requests them. – underscore_d Jul 12 '17 at 15:52
  • i.e. don't waste resources on dynamic allocation and/or pointer semantics if there is no specific purpose to any of it. – underscore_d Jul 12 '17 at 15:59
  • have you tried return std::move(mUnits) ? – Zbigniew87 Jul 12 '17 at 15:36

1 Answers1

23

A value returned in C++ incurs a copy - if that's what you want, you can't use unique_ptr or have to make a manual copy and std::move it. However, I take you only want to provide access to the Units (for whatever reason...), so simply return a reference:

std::vector<std::unique_ptr<Unit>> const& getUnits() const
{
  return mUnits;
}

(The const is only needed if you don't want to give write access into the vector.)

Now, a few questions arise still: Why is ~Tile virtual? I see no need.

Why do you use an owning ptr to point to the units? In the original Java code, you get handed a reference to an external Unit and only store a reference - if the Tile in the Java code is destroyed, the units are not (from what I can see), as long as they are referenced somewhere else. To achieve the exact same in C++, you were correct in using shared_ptr.

Of course, you could just clarify who is the real owner of the units and operate accordingly - if it is not Tile, use a raw pointer. See also "Which kind of pointer do I use when?"

Community
  • 1
  • 1
Xeo
  • 129,499
  • 52
  • 291
  • 397
  • 3
    `~Tile()` is virtual because the Java code has `protected` methods, implying that the intention is to derive classes from this one. – Pete Becker Sep 06 '12 at 14:58
  • Funny... I tried auto test = getUnits() but I got an error: "use of deleted function 'std::unique_ptr etc etc :-( – darkman Sep 06 '12 at 16:22
  • 6
    @darkman: Well.. `auto x = init;` will cause a copy to happen. Use `auto& x = init;`. – Xeo Sep 06 '12 at 16:29
  • Why is this not working in Visual Studio 2013? I still get a compiler error trying to access copy constructor... – tomi.lee.jones Jan 22 '15 at 17:45
  • In "auto& x = init;", Xeo use & after auto to specify x is an lvalue reference to init. Check here for further detail: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2027.html – Yingpei Zeng May 23 '17 at 03:02