Back to General and Gameplay Programming

Virtual function design question

General and Gameplay Programming Programming

Started by SimonFarris November 05, 2009 09:04 AM

15 comments, last by RandomBystander 14 years, 5 months ago

SimonFarris

122

Author

November 05, 2009 09:04 AM

There's a problem I've encounted several times and I've never been very happy with the solutions I've used, so I figured that I would see if anyone has a better solution. So I have a base class A:

class A
{
	virtual int func( )=0;
}

and then classes B-H that all derive from A and implement func(). I have some function that calls func using an A pointer, a vector of A pointers, or something else like this:

void somefunc()
{
	for ( i=0; i<avector.size( );i++ )
	{
		A* a=avector;
		int result=a->func( );
		//use result or something
	}
}

The problem occurs when I want to add a class I, but its result depends on some other data X within somefunc(). There are two ways I've solved this in the past: Using a type check to call I's func() differently:

void somefunc()
{
	X x;
	for ( i=0; i<avector.size( );i++ )
	{
		A* a=avector;
		int result=0;
		if ( a->isI( ) )
			result = static_cast<I*>( a )->func( x );
		else
			result = a->func();

		//use result or something
	}
}

or change A-H so that func() takes an X parameter, but B-H all just ignore it:

void somefunc()
{
	X x;
	for ( i=0; i<avector.size( );i++ )
	{
		A* a=avector;
		int result=a->func( x );
		//use result or something
	}
}

Both of these options work, but they feel ugly to me. Option 1 scales a little better if I end up having to add something like class J with func( Y y ) later. Option 2 is better from an OOP perspective, but it feels wrong to modify A-H just to make I work. Does this situation occur to anyone else? Do you solve it differently? Is there a better way to re-architect this? Thanks, Simon

scjohnno

236

November 05, 2009 09:10 AM

I doesn't conform to the interface specified by A, so should I really be an A? Perhaps you can define a new interface, and have somefunc() operate on it separately.

jpetrie

13,220

November 05, 2009 10:23 AM

This generally means that the interface of A is not sufficient for what you're applying it to. It's harder to say more without additional detail and context, but a better solution is almost always to change the overall design -- changing the interface, adding a different one, changing how you operate on those interfaces, et cetera -- than to try and force an improper usage into the framework of the existing design.

Antheus

2,410

November 05, 2009 10:27 AM

Quote:Original post by SimonFarris
There's a problem I've encounted several times

Instead of posting the solution - what was the problem?

SimonFarris

122

Author

November 05, 2009 11:53 AM

Quote:Original post by scjohnno
I doesn't conform to the interface specified by A, so should I really be an A? Perhaps you can define a new interface, and have somefunc() operate on it separately.

Sorry, I should have made that clearer. func() is only a small part of the A class, imagine that there are a dozen other virtual functions that are overridden by the other classes, including I. The only thing unique about I is that one of the dozen functions on it needs a bit more data then any of the other classes requires. There's obviously some refactoring required, I'm just looking for a nicer way than the two I posted, if there is one.

Quote:Original post by jpetrie
This generally means that the interface of A is not sufficient for what you're applying it to. It's harder to say more without additional detail and context, but a better solution is almost always to change the overall design -- changing the interface, adding a different one, changing how you operate on those interfaces, et cetera -- than to try and force an improper usage into the framework of the existing design.

That's pretty much the heart of my post I think. The second solution appears to be the most correct of the two, where the interface for A is changed and B-H are updated to use the new interface, but ignore the X argument. It gets a bit more complicated if another new class with a unique requirement is needed, like J::func( Y y ). In that case the first solution requires less work, but is uglier.

I'm 100% in favor of changing the overall design in order to deal with the new requirements for I(and potentially J), I just don't know what options there are other than the two I posted.

Quote:Original post by Antheus
Instead of posting the solution - what was the problem?

The problem is that I need to add class I, but it has some additional requirements that A-H do not have. Is there a better way to refactor the hierarchy than the two ideas I posted?

jpetrie

13,220

November 05, 2009 12:01 PM

Quote:
The second solution appears to be the most correct of the two, where the interface for A is changed and B-H are updated to use the new interface, but ignore the X argument. It gets a bit more complicated if another new class with a unique requirement is needed, like J::func( Y y ). In that case the first solution requires less work, but is uglier.

The first option is almost never ideal. The second solution is better, but still subpar. The fact that one subclass needs extra data that the rest will ignore suggests that that subclass shouldn't be subclassed from the base. You can't treat it the same, so it isn't the same. But this depends on the actual problem you're trying to solve -- what I, X, A, B and J (et cetera) really represent.

Quote:
I'm 100% in favor of changing the overall design in order to deal with the new requirements for I(and potentially J), I just don't know what options there are other than the two I posted.

There are hundreds of thousands of them. Only a subset of them apply however -- it depends very much on the actual problems you're trying to solve. Your examples are too abstract to be of use to you.

Quote:
The problem is that I need to add class I, but it has some additional requirements that A-H do not have. Is there a better way to refactor the hierarchy than the two ideas I posted?

I think Antheus was getting at the same thing I am: we need some additional contextual information, some information about what these useless letters represent. This is an OO design problem, and to deal with it effectively you need to speak in the vocabulary of the objects.

RandomBystander

122

November 05, 2009 07:15 PM

While waiting for more context...
Basically, class I needs some extra information via class X to fulfill the promise given by A::func(). Instead of pushing this information, let class I simply pull it.

class X {   private:   static X* last_created;   public:   static X* get_last_created()    {      if (last_created==0) throw PleasePanicNow("HAND");      return last_created;   }   X() { last_created=this;}   ~X() { if (last_created==this) last_created=0;}};class I {   public:   int func()    {       X* x=X::get_last_created();       // do something   }};void somefunc(){	X x;	for ( i=0; i<avector.size( );i++ )	{		A* a=avector;		int result=a->func();                /* mysterious stuff happens */	}}

As far as I can see, this code has the same behaviour as yours - unless the constructor of X is called at an inopportune moment.

SimonFarris

122

Author

November 05, 2009 08:01 PM

Quote:Original post by jpetrie
I think Antheus was getting at the same thing I am: we need some additional contextual information, some information about what these useless letters represent. This is an OO design problem, and to deal with it effectively you need to speak in the vocabulary of the objects.

Ok then if specifics are needed. The most recent example of this problem that I ran into was for the items in a shop. A is the base class for the item, and B-H are various different kinds of items. A has many different virtual functions that B-H all implement. The function func() from the example is:

virtual int getprice( Person* owner );

Each item type uses the owner and its own data to determine its price.

The new class that I'm trying to add(I) is an item type where the price depends on the time of day or the day of the week. In order to do that, the getprice() function needs to have access to that information.

Another example of a class(J) would be a type of item where I want some aspect of the buyer to influence the price, so getprice() would need access to that.

With those two new classes, my getprice() function either becomes:

virtual int getprice( Person* owner, Person* buyer, int dayofweek );

and all the other classes just ignore buyer and dayofweek, or I put the special cases at the point where I call getprice().

Obviously, I don't really like either of those options. If there's no other way to restructure things or set this scenario up I can accept that. I just hope there's some better way.

Thanks again,

Simon

Beyond_Repair

116

November 05, 2009 08:22 PM

Quote:Original post by RandomBystander
While waiting for more context...
Basically, class I needs some extra information via class X to fulfill the promise given by A::func(). Instead of pushing this information, let class I simply pull it.
*** Source Snippet Removed ***
As far as I can see, this code has the same behaviour as yours - unless the constructor of X is called at an inopportune moment.

Aside from being fairly WTF at first sight, that code isn't reentrable and probably is very hard to debug.

Antheus

2,410

November 05, 2009 08:27 PM

While at first letting item determine its worth seems like a good idea, it really isn't. After all - when was the last time you asked your coffee cup how much it costs. The best item can provide is "worth", some abstract unit of its value.

In typical economy, actual price will be determined by owner and usually seller.

This means:

struct Seller {  virtual int getPrice(Item * i, Person * buyer);};

What about other properties? Implementation detail.

Shopkeeper will be somehow placed in world, so they'll be able to know the time.

struct Shopkeeper : Person, Seller {  int getPrice(Item * i, Person * buyer) {    if (i->owner() != this) // not ours to sell    if (buyer->getFaction() != this->getFaction()) // we don't like them    int multiplier = 1;    if (this->getWorld()->getDay() == 2) multiplier = 2; // double price on Tuesday    return item->worth() * multiplier;  }};

Or something similar.

Virtual function design question

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Virtual function design question

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines