Defining shared class member functions only once

[Phlucious]

I couldn't really figure out how to form the question in a way that makes sense in a search engine, so if this has been asked already by someone else I apologize.

I have a set of classes, some of which have a member function, setRGB(quint16, int) for example, that does the exact same thing. I'd like to clean up my code so that setRGB is only defined once without resorting to external functions because it accesses private variables.

They all inherit from an abstract core class. For clarity's sake I don't want them to inherit from each other, so inheritance isn't really an option. I'm open to the idea of templates, but as I have never designed one before, I'm not sure when it's appropriate to use that concept.

Here's an example. A, B, and C all inherit from CORE:

//stuff.h
//use a shortcut macro to reduce error from copy-pasting
#define PTRGB \
public: \
quint16 setRGB(quint16 v, int i); \
quint16 getRGB(int i) const {return rgb[i]}; \
private: \
quint16 rgb[3];

class CORE
{
public:
virtual quint16 setRGB(quint16 v, int i);
virtual quint16 getRGB(int i) const {return 0;}
//other functions and variables common to all of my classes
};

class A : public CORE
{
PTRGB
private:
int extraVariable;
}

class B : public CORE
{
//doesn't have setRGB implemented!
private:
double aDifferentExtraVariable;
public:
double getSpecialB() {return aDifferentExtraVariable;}
}

class C : public CORE
{
PTRGB
private:
bool isBlue;
public:
bool getIsBlue() {return isBlue;}
}

//stuff.cpp
quint16 CORE::setRGB(quint16 v, int i)
{
return 0;
}

quint16 A::setRGB(quint16 v, int i)
{
#ifdef DEBUGMODE
if(i < 0 || i > 2)
{
qWarning() << "Index to setRGB is out of range.";
return 0;
}
#endif
return rgb[i] = v;
}

quint16 C::setRGB(quint16 v, int i)
{
#ifdef DEBUGMODE
if(i < 0 || i > 2)
{
qWarning() << "Index to setRGB is out of range.";
return 0;
}
#endif
return rgb[i] = v;
}
As far as I can tell, I can't do a similar macro for the definitions like I did with the declarations because of the class name in the "quint16 C::setRGB(quint16 v, int i)" lines, but I'm looking for similar functionality.

Any ideas?

[dancas2]

I was just looking for the exact thing you're trying to achieve, but I haven't got any good answer.. Actually I share those private members through a special class constructor and I define my functions in that class. I don't want to share those members, but it seems it is impossible without doing it. Probably the best I can do is passing a reference to the member each time I need to use it rather than storing that reference in a pointer.

Greetings!

[stampede]

A, B, and C all inherit from CORE:
You can introduce another class that extends CORE, lets say CORE_RGB, so it'll look like this:


//stuff.h

//use a shortcut macro to reduce error from copy-pasting

#define PTRGB \

public: \

quint16 setRGB(quint16 v, int i); \

quint16 getRGB(int i) const {return rgb[i]}; \

private: \

quint16 rgb[3];



class CORE

{

public:

virtual quint16 setRGB(quint16 v, int i);

virtual quint16 getRGB(int i) const {return 0;}

//other functions and variables common to all of my classes

};

class CORE_RGB : public CORE{
PTRGB
// there is no need for the macro now btw
};

class A : public CORE_RGB

{
private:

int extraVariable;

}



class B : public CORE

{
//doesn't have setRGB implemented, because it extends CORE, not CORE_RGB
private:

double aDifferentExtraVariable;

public:

double getSpecialB() {return aDifferentExtraVariable;}

}



class C : public CORE_RGB

{
private:

bool isBlue;

public:

bool getIsBlue() {return isBlue;}

}

In the end, all A,B,C classes extends CORE as you wanted, and RGB methods are defined only in one place.

[dancas2]

It is probably the best idea that can work for you, Phlucious. I was thinking it could work for me, but, what if the needed variables are part of UI? I would have to create private variable pointers because I have no access to the ui pointer from the base.

PD: Should I create a new thread?

Greetings

[Phlucious]

Thanks for the suggestion, Stampede. Something along those lines will probably work, but here's an added complication. Each class has a different combination of about 5 options, one of those being RGB as in the above example. For example, class A has RGB & TIME, class B has only TIME, class C has RGB & ALPHA, class D has ALPHA & TIME, etc.

Basically what I'm talking about is akin to Qt's property system, except there's about 10 classes that mix & match which properties they actually have... each property with a custom get/set function. So the problem with extending CORE is that not all classes with TIME also have RGB (for example).

Any suggestions or further comments?

[dancas2]

You could create a class for RGB, a class for TIME, a class for ALPHA, and then inherit from every one you need.

Greetings

[Phlucious]

You could create a class for RGB, a class for TIME, a class for ALPHA, and then inherit from every one you need.
While this idea is sound in theory, it doesn't quite work in my current execution. I store in a std::vector about 2-20 million abstract CORE* pointers that point to an instantiation of A, B, C, etc. I can do this since all of the functions in ABCD exist as a prototype in CORE.

Therefore, I'm not sure what would happen if, for example, CORE_RGB & CORE_TIME inherited from CORE, and A inherited from both CORE_RGB & CORE_TIME.

[amleto]

why cant

quint16 rgb[3]

be protected and in CORE?



Therefore, I'm not sure what would happen if, for example, CORE_RGB & CORE_TIME inherited from CORE, and A inherited from both CORE_RGB & CORE_TIME.

Currently, nothing bad, since CORE is only an interface. If you put members variables into CORE, then you need to inherit CORE virtually, to avoid the dreaded diamond



I can do this since all of the functions in ABCD exist as a prototype in CORE.

This begs the question, why do all types implement all members? It looks the the change in behaviour hasnt be encapsulated properly.

[Phlucious]

Thanks for your response!


why cant
quint16 rgb[3]
be protected and in CORE?
Normally I would just make a class that encapsulates every possible combination and be done with it, drastically simplifying my code. The problem rests in the problem of scale—when you have an extra 60 bytes (more or less) per record floating around, and around 6,000,000 records, that's an extra 343MB that I need to drag around. I'm already pushing the available memory as it is, so my intent is to only include RGB/TIME/ALPHA when the records actually have that data.


If you put members variables into CORE, then you need to inherit CORE virtually, to avoid the dreaded diamond.
Can you please elaborate on this? I'm not a programmer by training, so I'm unfamiliar with some of the lingo. Also, what do you mean by inheriting it virtually? I've never been clear on the functional distinction between Protected and Private. If I make the prototyped member functions of CORE protected, then my generalized manipulation functions (using CORE* pointers, though the object pointed to will be A, B, C, or D depending on the data imported) throw a compiler error.

[marcvanriet]

Hi,

You could create a 'class' or 'type' RGB which has public members r, g and b. It would have a method setRGB() to set these three members, and also the other member functions you mention.

The A and C classes would have a member of type RGB, say m_rgb. If a method in this class would need to know anything about the color, it would use the members m_rgb.r, m_rgb.g and m_rgb.b.

This way you don't have to implement anything in your base classes, or in any other class in which you would like to use a color.

Best regards,
Marc

[amleto]

Thanks for your response!


Normally I would just make a class that encapsulates every possible combination and be done with it, drastically simplifying my code. The problem rests in the problem of scale—when you have an extra 60 bytes (more or less) per record floating around, and around 6,000,000 records, that's an extra 343MB that I need to drag around. I'm already pushing the available memory as it is, so my intent is to only include RGB/TIME/ALPHA when the records actually have that data.


Can you please elaborate on this? I'm not a programmer by training, so I'm unfamiliar with some of the lingo. Also, what do you mean by inheriting it virtually? I've never been clear on the functional distinction between Protected and Private. If I make the prototyped member functions of CORE protected, then my generalized manipulation functions (using CORE* pointers, though the object pointed to will be A, B, C, or D depending on the data imported) throw a compiler error.

inheriting virtualy is not to do with access level (public/private/protected).

If you have some base class B, and class A and class C both inherit B, then some other class, D is designed that wants to inherit A and C we have a problem IF B has data members.



B
/\
A C
\/
D

If B has data members, then they exist as part of A, and ALSO as part of C. This is a big problem. To get around this, inherit B virtually (use virtual keyword like virtual method). Then if some class has multiple instances of B, only one will actually be made.


However, since you say you can't use the members in the base class, you wont have this problem.

[Phlucious]

Thank you for the great suggestion! Inheriting virtually seems to have the intended results. Following your diamond structure...


class B {}
class A : virtual public B {}
class C : virtual public B {}
class D : public A, public C {}

Unfortunately, I get the C4250 "[derived class] inherits [overwritten method in derived class] via dominance" warning now for every virtual function I re-defined. I understand that this warning is for programmers that might be caught by surprise, but is there any syntactic way for me to indicate to MSVC2010 that I'm doing this on purpose?


I found this link useful for understanding what you wrote:
http://publib.boulder.ibm.com/infocenter/lnxpcomp/v8v101/index.jsp?topic=%2Fcom.ibm.xlcpp8l.doc%2Flanguage% 2Fref%2Fcplr135.htm

[amleto]

this warning is a bit different - as long as you have read and undersand what it warns against, and that if you have two reimplemented virtuals at the same inheritance level, then that would be an error.

you can supress warnings with #pragma or project settings

http://msdn.microsoft.com/en-us/library/2c8f766e%28v=vs.80%29.aspx

[marcvanriet]

Still, I think multiple inheritance is not the right solution for your initial problem.

Say you have a class 'bike', and if you have a class 'livingroom'. Both have a light that you can switch on and off. That doesn't mean it is a good idea to have a class 'light' and derive 'bike' and 'livingroom' from it.

Given your level of expertise in C++, it might be better to keep things simple. Plain old library functions are still widely used. It is not because C++ supports object oriented design, that everything has to be implemented in an object-oriented fashion. And static 'utility' functions are also widely used for a class where the function doesn't work on an object itself, but sort of belongs to the class anyway.

Regards,
Marc

[wysota]

Say you have a class 'bike', and if you have a class 'livingroom'. Both have a light that you can switch on and off. That doesn't mean it is a good idea to have a class 'light' and derive 'bike' and 'livingroom' from it.
What if you call the class 'Lightable' and not 'light'?

class Lightable {
public:
virtual void enableLight() { ... }
virtual void disableLight() { ... }
};

class Bike : public Vehice, public Lightable { ... };
class LivingRoom : public Chamber, public Lightable { ... };
Sounds reasonable to me...


An alternative is to have an external helper class that implements solely the "shared" functionality and call that object's methods passing the object it is to manipulate. Then you need implementations of that class to handle objects of specific types (ones that have the "RGB" functionality and those of the "TIME" functionality). Patterns like 'visitor' or 'strategy' come to my mind here but probably more solutions are possible and viable.
In the end you have one "instance" of functionality per class instead of having one per item which escapes the memory hog trap.

[marcvanriet]

What if you call the class 'Lightable' and not 'light'?
Sounds reasonable to me...


Sure, but all depends on the situation. What if you then come across a bike of which the front light can be set to three states : on - blinking - off. And when there are multiple independently controlled lights in the chamber ? Unfortunately the OP didn't post enough details of the use case to say if each object can really have only 1 color. Maybe later on there could be objects that have a 'front' and 'back' color.

I would probably prefer an interface like
bike1->frontlight->enableLight();
room1->ceilinglight->disableLight();
room1->curtainlights->enableLight();
If the 'frontlight' and so have a reference to the class of which they are member, events can be triggered when a light status changes if this is necessary.

But again... all depends on the use case.

Regards,
Marc

[wysota]

Sure, but all depends on the situation. What if you then come across a bike of which the front light can be set to three states : on - blinking - off. And when there are multiple independently controlled lights in the chamber ?
Then you extend the Lightable interface. What you say applies to any class, inheritance, single or multiple, is irrelevant here. If you continue this way, you'll reach a conclusion that there is no point in creating classes at all because some day you might want to change something in your program breaking what you did before.




I would probably prefer an interface like
bike1->frontlight->enableLight();
room1->ceilinglight->disableLight();
room1->curtainlights->enableLight();
If the 'frontlight' and so have a reference to the class of which they are member, events can be triggered when a light status changes if this is necessary.


This changes nothing really. You still have to be able to know that some object has a "frontlight", "ceilinglight" and "curtainlight" members and that some other has none of them. The point is to have a common base class and then say "if this object has a ceiling light, turn it on". For that you can use double dispatch.

Or in terms of C++:


class Visitor {
public:
virtual void visitLivingRoom(LivingRoom *) {};
virtual void visitBike(Bike *) {};
};

class Item {
public:
virtual void accept(Visitor &visitor) = 0;
};

class LivingRoom : public Item {
public:
void accept(Visitor &visitor) { visitor.visitLivingRoom(this); }
};

class Bike : public Item {
public:
void accept(Visitor &visitor) { visitor.visitBike(this); }
};

And then:


class TurnLightsOnBikes : public Visitor {
public:
void visitBike(Bike *bike) { bike->turnLightsOn(); }
};


QList<Item*> items = ...
TurnLightsOnBikes bVisitor;
foreach(Item *item, items) {
item->accept(bVisitor);
}

Since C++ doesn't have double dispatch (only single dispatch through virtual methods) it has to be emulated like in the code above. Furthermore the visitor can be made a friend of Item subclasses to allow access to private members.

In the "adding more lights" case it is enough to modify the visitor or create a new one. When a new subclass of Item is added, it is enough to add a new method to the Visitor interface. In all cases the Item class API remains clean and functionality is added by implementing a new Visitor subclass (which gives a nice separation and extendability).

[marcvanriet]

Phlucious: I'm not a programmer by training, so I'm unfamiliar with some of the lingo.

Wysota, I think we've lost someone in the discussion here.. :)

Maybe my example with the bike and chamber didn't really fit the OP's original problem. That's the danger of an OP asking a generic question without a real-life example. And that's also the danger for a replier of using a counter-example that is taken in other directions then intended.

Anyway, what I mean to say is that OOP techniques aren't necessarely ALWAYS the best techniques for handling a problem. Especially for a novice. Can we agree on that ? :)

Thanks for the pointer to the 'visitor' pattern. Didn't know that, will study it in more detail.

Regards,
Marc

[wysota]

Anyway, what I mean to say is that OOP techniques aren't necessarely ALWAYS the best techniques for handling a problem. Especially for a novice. Can we agree on that ? :)

I think multiple inheritance is a viable approach here.

[marcvanriet]

OK, you win :)

[wysota]

You have the right to have your own opinion, as do I to have mine. Nobody wins or loses here.

[marcvanriet]

I know. Notice the smiley in the last post :)

I just wanted to acknowledge your superiority in the matter. After all, I'm just an electronics guy that does some embedded and PC software programming too. :)

Notice the smiley again :)

Happy new year by the way !

[Phlucious]

An alternative is to have an external helper class that implements solely the "shared" functionality and call that object's methods passing the object it is to manipulate. Then you need implementations of that class to handle objects of specific types (ones that have the "RGB" functionality and those of the "TIME" functionality). Patterns like 'visitor' or 'strategy' come to my mind here but probably more solutions are possible and viable.

In the end you have one "instance" of functionality per class instead of having one per item which escapes the memory hog trap.

I'm intrigued by this possibility. Are you basically describing the situation of having specialized storage classes with one generalized access class?

[wysota]

More like having specialized functionality classes with one generalized structure/access class.