Jump to content
  • Advertisement
Sign in to follow this  
SimonFarris

Virtual function design question

This topic is 3178 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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

Share this post


Link to post
Share on other sites
Advertisement
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Quote:
Original post by SimonFarris
There's a problem I've encounted several times


Instead of posting the solution - what was the problem?

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!