views:

319

answers:

4

I'm writing a simplistic game to learn get some more C++ experience, and I have an idea where I feel polymorphism almost works, but doesn't. In this game, the Party moves fairly linearly through a Map, but can occasionally encounter a Fork in the road. A fork is (basically) an std::vector<location*>.Originally I was going to code something like the following into the a Party member function:

if(!CurrLocation->fork_.empty())
   // Loop through forks and show options to the player, go where s/he wants
else
  (CurrLocation++)

But I was wondering if some variant of the following might be better:

CurrLocation = CurrLocation->getNext();

With Fork actually being derived from Location, and overloading some new function getNext(). But in the latter case, the location (a low level structure) would have to be the one to present the message to the user instead of "passing this back up", which I don't feel is elegant as it couples location to UserInterface::*.

Your opinions?

+1  A: 

You should use polymorphism as long as it makes sense and simplifies your design. You shouldn't use it just because it exists and has a fancy name. If it does make your design simpler, then it's worth the coupling.

Correctness and simplicity should be the ultimate goal of every design decision.

Mehrdad Afshari
A: 

Polymorphism does not lend to greater coupling, I think they are separate issues.

In fact if you are programming to interfaces and following general inversion of control patterns then you will lead to less or zero coupling.

In you example, I don't see how location is coupled to UserInterface?

If this is the case, can the coupling be removed through another level of abstraction in between UserInterface and Location, such as LocationViewAdapter?

Xian
Inheritance can increase coupling, and polymorphism tends to depend on inheritance.
David Thornley
True, but if you are injecting your base classes into your methods then this is way to reduce your coupling.
Xian
+1  A: 

I think you spotted the issues yourself and can probably work it out with your knowledge of the rest of the system or a few more details here for us to look at.

As has been mentioned:

  1. Polymorphism should be used to simplify the design - which it would do in this case, so well spotted.
  2. You have an issue with the coupling - again well spotted, coupling can lead to problems later on. However, what this says to me is that the way in which you are applying the polymorphism might not be the best way.
  3. Programming to interfaces should allow you to hide the internal details of how the system is put together and so reduce the coupling.
Big GH
+6  A: 

All problems can be solved by adding a level of indirection. I would use your suggested variant, and decouple Location from Party by allowing getNext to accept an object that resolves directional choices. Here is an example (untested):

class Location; 

class IDirectionChooser
{
public:
  virtual bool ShouldIGoThisWay(Location & way) = 0;
};

class Location
{
public:
  virtual Location * GetNext(IDirectionChooser & chooser)
  {
    return nextLocation;
  }

  virtual Describe();
private:
  Location * nextLocation;
};

class Fork : public Location
{
public:
  virtual Location * GetNext(IDirectionChooser & chooser)
  {
    for (int i = 0; i < locations.size(); i++)
      if (chooser.ShouldIGoThisWay(*locations[i]))
        return locations[i];
  }
  virtual Describe();
private:
  vector<Location *> locations;
};

class Party : public IDirectionChooser
{
public:
  void Move()
  {
    currentLocation = currentLocation->GetNext(GetDirectionChooser());
  }

  virtual IDirectionChooser & GetDirectionChooser() { return *this; }

  virtual bool ShouldIGoThisWay(Location & way)
  {
    way.Describe();
    cout << "Do you want to go that way? y/n" << endl;

    char ans;
    cin >> ans;
    return ans == 'y';
  }
};
SCFrench
Pity about the IInterface nomenclature. Perfect otherwise.
ewalshe
Really? Why? We use it where I work. If there is a good reason not to use it let me know and I'll edit the answer.
SCFrench
Nice! I would suggest adding to Party: "virtual IDirectionChooser }", and having Move() call this instead of directly using *this -- this will enable a subclass to use a different (e.g. external) IDirectionChooser implementation by overriding just that method.
j_random_hacker
Updated based on j_random_hacker's comments.
SCFrench