1

I'm having trouble passing a derived class to a function which accepts the base class as argument. The base class is consists of "obstacles" which are to be placed on a "board" void Board::setvalue(int length, int width, Obstacle& obstacle);

However, this causes the compiler to give the "no known conversion for argument..."-error. Reading up around the site i found that i should be passing the derived object as a const, this however causes problems because a const can't be assigned to the board (since it holds pointers to non-const Obstacles).
In turn, changing Board to hold const Obstacles causes a lot of issues elsewhere in the project, especially with the operator<< of Board and Obstacle.
I have tried passing the objects as consts and then using Obstacle ob = new obstacle(the const obstacle) but this made them generic Obstacle objects rather than Player/Barrel/Wall objects.

Is there any way to pass these objects as non-consts or assigning them as non-consts? i tried using const_cast() but this caused undefined behaviour.

An example of the function call:

Board_->setvalue(x, y, Player(data, moveable, x, y));

Here is my code:

The base class

class Obstacle
{
    public:
    Obstacle* _properlyinitialized;
    string Name;
    bool Moveable;
    int x;
    int y;
    Obstacle();
    Obstacle(string Name, bool Moveable, int x, int y);
    virtual ~Obstacle();
    bool properlyInitialized();
    friend std::ostream& operator<<(std::ostream& stream, Obstacle& Obstacle);
};

An example of the derived classes (other derived classes don't have special functions yet)

class Player: public Obstacle
{
public:
    Player():Obstacle(){};
    Player(string Name, bool Moveable, int x, int y):Obstacle(Name, Moveable, x, y){this->_properlyinitialized = this;};
    ~Player(){};
    /*void Moveleft();
    void Moveright();
    void Moveup();
    void Movedown();*/
};

The Board class header

class Board
{
private:
    Board* _properlyinitialized;
    int length;
    int width;
    Obstacle * * * playfield;

public:
    /*
     **ENSURE(this->properlyInitialized(),
                "Object wasn't initialized when calling object");
     */
    Board();
    Board(int length, int width);
    ~Board();
    bool properlyInitialized();
    /*
     **REQUIRE(this->properlyInitialized(),
            "Object wasn't initialized when calling properlyinitialized");
     */
    void clear();
    const int getLength();
    const int getWidth();
    Obstacle*** getBoard();
    Obstacle* getTile(int length, int width);
    void setvalue(int length, int width, Obstacle& obstacle);
    friend std::ostream& operator<<(std::ostream& stream, Board& Board);
};

std::ostream& operator<<(std::ostream& stream, Board& Board);

And finally, the setvalue function.

void Board::setvalue(int length, int width, Obstacle& obstacle)
{
    this->playfield[length][width] = &obstacle;//value;
    return;
}

I'm happy to provide more code if needed.

Felix Neijzen
  • 37
  • 8
  • 3
    less and more concise code would be better. – davidhigh Jun 05 '15 at 11:07
  • I think you Need to work with pointers. This makes it possible to Keep the original class. – Thomas Sparber Jun 05 '15 at 11:10
  • @davidhigh I tried cleaning it up somewhat, separated the sections better and removed the other derived classes since they're practically duplicates at this point. Does it look better now? – Felix Neijzen Jun 05 '15 at 11:31
  • @FelixNeijzen: good, sure it looks better now. But for a real minimal example, you can leave all constructors, destructors, unrelated getters, setters and data members aside (then you could end up with 20 lines of code). – davidhigh Jun 05 '15 at 11:40

2 Answers2

2

Instead of a complete code review (-- which is not what SO is for), let's get directly to the routine you mentioned 

void Board::setvalue(int length, int width, Obstacle& obstacle)
{
    this->playfield[length][width] = &obstacle;
    return;
}

which sets a triple pointer

Obstacle *** playfield;

This design is bad for several reasons, but here is the main one: it is not clear at all that the ostacle is still alive when you want to call it via Board::playfield. Nobody ensures that player isn't long destroyed, and you will be having a hard time in bookkepping this fact.

Instead, I suggest you to let the board own the obstacles. Thus, instead of an obstacle raw pointer, set up a vector of unique-pointers,

std::vector<std::unique<Obstacle> > playfield;

and then either copy or move the classes:

template<typename O>
void Board::setvalue(int length, int width, O&& obstacle)
{
    playfield.push_back(std::make_unique<O>(std::forward<O>(obstacle));
}

(I've left the field geometry aside, I doubt that it is useful to intermix it with the actual storage of the obstacles -- but if you still want to you can use a vector of vectors or a single vector with a two-dimensional index scheme).

And here back to your intention: With the above approach, you directly get rid of all constness problems. You aka. the Board owns the stuff and can do with it what you want.

davidhigh
  • 14,652
  • 2
  • 44
  • 75
  • good solution. But std::make_unique isn't it only c++14 ?. May be a little tricky to compile. std::make_shared could be enought for what he's trying to do. – Bastien Jun 05 '15 at 11:31
  • @Bastyen: yes, it's a C++14 feature. Here is the C++11 workaround [http://stackoverflow.com/a/12580468/2412846] – davidhigh Jun 05 '15 at 11:33
  • @FelixNeijzen: yes, it's clear that you didn't ask for it. Still, there are other thing which I would question if this were [codereview](http://codereview.stackexchange.com/) ... like the `Board* _properlyinitialized;`and others. (If you're interested, I'd encourage you to go there and ask a question on your code). – davidhigh Jun 05 '15 at 11:45
  • wasn't looking for a code review (i hoped that was clear), there is still a bunch of other code. I went with this approach instead of vectors because vectors are less easy to work in when working with specific items at a time. in this case i could use X/Y coordinates. i also wasn't really sure what to do with the empty fields. I'll try it though, it does look a lot cleaner as far as desgin is concerned – Felix Neijzen Jun 05 '15 at 11:45
  • The _properlyinitialized is a requirement to be included in the project (it's a university project), i'm actually not entirely fond of it either. – Felix Neijzen Jun 05 '15 at 12:02
0

The problem here is that you try to pass a const value (Player(data, moveable, x, y)) as reference. You cannot do that. Regarding the fact that you store the object in your array playfield, you should definitively use a pointer or much better, a shared_ptr and store it in a std::list or std::vector, to avoid delete problems.

Bastien
  • 994
  • 11
  • 25