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.

4
  • 3
    less and more concise code would be better. Commented Jun 5, 2015 at 11:07
  • I think you Need to work with pointers. This makes it possible to Keep the original class. Commented Jun 5, 2015 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? Commented Jun 5, 2015 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). Commented Jun 5, 2015 at 11:40

2 Answers 2

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.

Sign up to request clarification or add additional context in comments.

5 Comments

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.
@Bastyen: yes, it's a C++14 feature. Here is the C++11 workaround [stackoverflow.com/a/12580468/2412846]
@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 ... like the Board* _properlyinitialized;and others. (If you're interested, I'd encourage you to go there and ask a question on your code).
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
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.
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.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.