feedback on C++ code snippet requested (standards compliance, efficiency, etc)

  • Thread starter christopher diggins
  • Start date
C

christopher diggins

I would like some feedback if the follow code is standard compliant, and are
there any obvious redundancies or inefficiencies? The purpose of the code is
to emulate the behaviour of a struct FuBar which implements an interface
InterfaceFuBar which contains two functions Fu and Bar. The goal is to do so
without resorting to virtual functions.

#include <iostream>
template<typename SELF> struct ITABLE_InterfaceFuBar {
void(SELF::*Fu)();
void(SELF::*Bar)();
};
struct InterfaceFuBar {
struct DUMMY_InterfaceFuBar {
void Fu() { };
void Bar() { };
};
struct DUMMY_ITABLE_InterfaceFuBar : public
ITABLE_InterfaceFuBar<DUMMY_InterfaceFuBar> { };
DUMMY_InterfaceFuBar* mpObject;
DUMMY_ITABLE_InterfaceFuBar* mpTable;
InterfaceFuBar(void* pObject = NULL, void* pTable = NULL) {
mpObject = (DUMMY_InterfaceFuBar*)pObject;
mpTable = (DUMMY_ITABLE_InterfaceFuBar*)pTable;
};
void Fu() { (mpObject->*(mpTable->Fu))(); };
void Bar() { (mpObject->*(mpTable->Bar))(); };
};
template<typename I, typename M> struct Implementor {
operator I() { return I(NULL, NULL); };
};
template<typename M> struct Implementor<InterfaceFuBar, M> {
operator InterfaceFuBar() {
static ITABLE_InterfaceFuBar<M> tmp = { M::Fu, M::Bar };
return InterfaceFuBar(this, (void*)&tmp);
};
};
struct FuBar : public Implementor<InterfaceFuBar, FuBar> {
void Fu() { std::cout << "fu" << std::endl; };
void Bar() { std::cout << "bar" << std::endl; };
};
int main() {
FuBar f;
InterfaceFuBar i = f;
i.Fu();
std::cin.get();
return 0;
}
 
R

red floyd

christopher said:
I would like some feedback if the follow code is standard compliant, and are
there any obvious redundancies or inefficiencies? The purpose of the code is
to emulate the behaviour of a struct FuBar which implements an interface
InterfaceFuBar which contains two functions Fu and Bar. The goal is to do so
without resorting to virtual functions.

[code redacted]

Short note. C style casts are evil. Use the C++ style casts instead.
 
L

llewelly

christopher diggins said:
I would like some feedback if the follow code is standard compliant, and are
there any obvious redundancies or inefficiencies? The purpose of the code is
to emulate the behaviour of a struct FuBar which implements an interface
InterfaceFuBar which contains two functions Fu and Bar. The goal is to do so
without resorting to virtual functions.

#include <iostream>
template<typename SELF> struct ITABLE_InterfaceFuBar {
void(SELF::*Fu)();
void(SELF::*Bar)();
};
struct InterfaceFuBar {
struct DUMMY_InterfaceFuBar {
void Fu() { };
void Bar() { };
};
struct DUMMY_ITABLE_InterfaceFuBar : public
ITABLE_InterfaceFuBar<DUMMY_InterfaceFuBar> { };
DUMMY_InterfaceFuBar* mpObject;
DUMMY_ITABLE_InterfaceFuBar* mpTable;
InterfaceFuBar(void* pObject = NULL, void* pTable = NULL) {
mpObject = (DUMMY_InterfaceFuBar*)pObject;
mpTable = (DUMMY_ITABLE_InterfaceFuBar*)pTable;
};
void Fu() { (mpObject->*(mpTable->Fu))(); };
void Bar() { (mpObject->*(mpTable->Bar))(); };
};
template<typename I, typename M> struct Implementor {
operator I() { return I(NULL, NULL); };
};
template<typename M> struct Implementor<InterfaceFuBar, M> {
operator InterfaceFuBar() {
static ITABLE_InterfaceFuBar<M> tmp = { M::Fu, M::Bar };
return InterfaceFuBar(this, (void*)&tmp);
};
};
struct FuBar : public Implementor<InterfaceFuBar, FuBar> {
void Fu() { std::cout << "fu" << std::endl; };
void Bar() { std::cout << "bar" << std::endl; };
};
int main() {
FuBar f;
InterfaceFuBar i = f;

You need to use the address-of operator:

InterfaceFuBar i = &f;

I'm too tired to examine your code for inefficiencies. Probably they
will be overwhelmed by the cost of your i/o.

i.Fu();
std::cin.get();
return 0;
}
[snip]
 
C

Claudio Puviani

christopher diggins said:
I would like some feedback if the follow code is standard compliant, and are
there any obvious redundancies or inefficiencies? The purpose of the code is
to emulate the behaviour of a struct FuBar which implements an interface
InterfaceFuBar which contains two functions Fu and Bar. The goal is to do so
without resorting to virtual functions.

#include <iostream>
template<typename SELF> struct ITABLE_InterfaceFuBar {
void(SELF::*Fu)();
void(SELF::*Bar)();
};
struct InterfaceFuBar {
struct DUMMY_InterfaceFuBar {
void Fu() { };
void Bar() { };
};
struct DUMMY_ITABLE_InterfaceFuBar : public
ITABLE_InterfaceFuBar<DUMMY_InterfaceFuBar> { };
DUMMY_InterfaceFuBar* mpObject;
DUMMY_ITABLE_InterfaceFuBar* mpTable;
InterfaceFuBar(void* pObject = NULL, void* pTable = NULL) {
mpObject = (DUMMY_InterfaceFuBar*)pObject;
mpTable = (DUMMY_ITABLE_InterfaceFuBar*)pTable;
};
void Fu() { (mpObject->*(mpTable->Fu))(); };
void Bar() { (mpObject->*(mpTable->Bar))(); };
};
template<typename I, typename M> struct Implementor {
operator I() { return I(NULL, NULL); };
};
template<typename M> struct Implementor<InterfaceFuBar, M> {
operator InterfaceFuBar() {
static ITABLE_InterfaceFuBar<M> tmp = { M::Fu, M::Bar };
return InterfaceFuBar(this, (void*)&tmp);
};
};
struct FuBar : public Implementor<InterfaceFuBar, FuBar> {
void Fu() { std::cout << "fu" << std::endl; };
void Bar() { std::cout << "bar" << std::endl; };
};
int main() {
FuBar f;
InterfaceFuBar i = f;
i.Fu();
std::cin.get();
return 0;
}

You're basically rebuilding C++'s virtual dispatch mechanism from scratch
inside InterfaceFuBar. Here's something that does the same thing as the code
you provided, but using C++'s existing virtual dispatch mechanism. You'll find
that this is no more expensive than the manual double indirection that's in the
code above.

class InterfaceFuBar
{
struct IFB_base
{
virtual void Fu() = 0;
virtual void Bar() = 0;
};

template <typename T>
struct IFB : public IFB_base
{
IFB(T & obj) : m_obj(obj) {}

virtual void Fu()
{
m_obj.Fu();
}

virtual void Bar()
{
m_obj.Bar();
}

T & m_obj;
};

IFB_base * m_pIFB;

InterfaceFuBar(); // disallow default init
InterfaceFuBar(const InterfaceFuBar &); // disallow copy init
InterfaceFuBar & operator=(const InterfaceFuBar &); // disallow copy

public:
template <typename T>
InterfaceFuBar(T & obj)
: m_pIFB(new IFB<T>)
{
}

~InterfaceFuBar()
{
delete m_pIFB;
}

template <typename T>
InterfaceFuBar & operator=(T & obj)
{
delete m_pIFB;
m_pIFB = new IFB<T>;
}

void Fu()
{
m_pIFB->Fu();
}

void Bar()
{
m_pIFB->Bar();
}
};

Claudio Puviani

P.S.: This code is for illustration purposes only. I didn't bother compiling or
testing it, it's not exception-safe, etc.
 
C

christopher diggins

----- Original Message -----
From: "Claudio Puviani" <[email protected]>
Newsgroups: comp.lang.c++
Sent: Thursday, April 15, 2004 10:26 PM
Subject: Re: feedback on C++ code snippet requested (standards compliance,
efficiency, etc)
Correction to my snippet:

// small detail: you left out the following line, apart from that it
compiles great
return *this;

Thank you very much for taking the time to work out an alternative solution.
For the example program I gave, your code performs correctly, and is only
somewhat slower to dispatch (it takes approximately twice as long to
dispatch the calls). The performance of the assignment of an object to your
InterfaceFuBar though is a big bottleneck due to the new and delete
operations. A small test showed that it took 50 times as long to assign to
your InterfaceFuBar when compared to the posted solution. Do you have
suggestions on how to accomplish the same thing without dynamic allocations?
In so doing would you be able to make InterfaceFuBar copy-able even if
different instances point to objects of different types? I like the elegance
of your solution and would like to be able to use something akin to it.
Thank you again for your help.

Christopher Diggins
http://www.cdiggins.com
http://www.heron-language.com
 
C

christopher diggins

Inspired by Claudio's ingenious design which did not require the superflous
inheritance statements I have made some changes to what I proposed earlier.
What I am especially concerned about in the below code are the following:
- Are the two typecasting structs are superflous?
- the GetStaticITable code seems to be clunky, is there a better way to
accomplish what it does?

#include<iostream>
using namespace std;
int naff = 0;
struct IFuBar {
// member struct for typecasting object
struct DUMMY_IFuBar {
void Fu() { };
void Bar() { };
};
// ITable type for this interface
template<typename SELF> struct ITABLE_IFuBar {
void(SELF::*Fu)();
void(SELF::*Bar)();
};
// member struct for typecasting ITable
struct DUMMY_ITABLE_IFuBar : public ITABLE_IFuBar<DUMMY_IFuBar> {
// empty
};
// member fields
DUMMY_IFuBar* mpObject;
DUMMY_ITABLE_IFuBar* mpTable;
// member functions
// default ctor
IFuBar() {
Assign(NULL, NULL);
}
// assignment ctor
template<typename T> IFuBar(T& o) {
Assign(&o, GetStaticITable<T>());
}
// copy ctor
template<> IFuBar(IFuBar& i) {
Assign(i.mpObject, i.mpTable);
}
// assignment from an arbitrary object that implements the interface
functiosn
template<typename T> IFuBar& operator=(T& o) {
Assign(&o, GetStaticITable<T>());
return *this;
}
// copy assignment
template<> IFuBar& operator=(IFuBar& o) {
Assign(o.mpObject, o.mpTable);
return *this;
}
// assignment implementation
void Assign(void* pObject, void* pTable) {
mpObject = (DUMMY_IFuBar*)pObject;
mpTable = (DUMMY_ITABLE_IFuBar*)pTable;;
}
// create static ITable for given object type
template<typename T> ITABLE_IFuBar<T>* GetStaticITable() {
static ITABLE_IFuBar<T> itable = { T::Fu, T::Bar };
return &itable;
}
// interface functions
void Fu() {
(mpObject->*(mpTable->Fu))();
}
void Bar() {
(mpObject->*(mpTable->Bar))();
}
};
struct FuBar {
void Fu() { naff = 0; };
void Bar() { naff = 1; };
};
struct BarFu {
void Fu() { naff = 2; };
void Bar() { naff = 3; };
};
int main() {
FuBar f;
BarFu b;
IFuBar x;
x = f;
x.Fu(); cout << naff << endl;
x.Bar(); cout << naff << endl;
IFuBar y = b;
x = y; // this line is crucial, it demonstrate that IFuBar does not need
the type of the object to work
x.Fu(); cout << naff << endl;
x.Bar(); cout << naff << endl;
cin.get();
return 0;
}
 
C

Claudio Puviani

christopher diggins said:
Thank you very much for taking the time to work out
an alternative solution.

Glad I could help.
For the example program I gave, your code performs
correctly, and is only somewhat slower to dispatch (it
takes approximately twice as long to dispatch the calls).

You'll find that the ratio will vary from compiler to compiler (and it will
decrease if you add arguments to the function), but yes, there is one extra
level of indirection.
The performance of the assignment of an object to your
InterfaceFuBar though is a big bottleneck due to the new
and delete operations. A small test showed that it took
50 times as long to assign to your InterfaceFuBar when
compared to the posted solution.

I have no doubt. I spent no time trying to make the example optimal.
Do you have suggestions on how to accomplish the same
thing without dynamic allocations?

Sure. Instead of dynamically allocating the IFB_base, keep a small, alligned
block of memory inside the InterfaceFuBar object and create the instance of
IFB_base in it using placement new. For example:

class InterfaceFuBar
{
// ... other stuff ...
union AlignmentHelper {
double d;
void * p;
};

AlignmentHelper m_memory[sizeof(IFB_base<InterfaceFuBar>) /
sizeof(AlignmentHelper) + 1];

// ... later ...
template <typename T>
InterfaceFuBar(T & obj)
{
new (m_memory) IFB<T>(obj);
}

template <typename T>
InterfaceFuBar & operator=(T & obj)
{
((IFB_base *)m_memory)->~IFB_base();
new (m_memory) IFB<T>(obj);
return *this;
}

void Fu()
{
((IFB_base *)m_memory)->Fu();
}

// ...
};

This should cut the price of creation and copying down to something more
reasonable.
In so doing would you be able to make InterfaceFuBar
copy-able even if different instances point to objects
of different types?

It's just a matter of exposing (and implementing) the copy constructor and
overloading operator= to take a const InterfaceFuBar &. You'd also want a
'placement' clone operation inside IFB to allow the cloning to happen in-place.
For example:

template <typename T>
struct IFB : public IFB_base
{
// ...
virtual void clone(void * pMem)
{
new (pMem) IFB<T>(*this);
}
// ...
};

Your copy ctor and supplemental operator= would then look like this:

InterfaceFuBar(const InterfaceFuBar & obj)
{
obj.clone(m_memory);
}

InterfaceFuBar & operator=(const InterfaceFuBar & obj)
{
if (this != &obj) {
((IFB_base *)m_memory)->~IFB_base();
obj.clone(m_memory);
}
return *this;
}
I like the elegance of your solution and would like to be able
to use something akin to it.

So long as you don't try patenting it (not that I claim to be the first to
'discover' it). ;-)
Thank you again for your help.

No problem. Thanks for turning the topic back to standard C++.

Claudio Puviani
 
C

christopher diggins

Claudio Puviani said:
[snip]
I like the elegance of your solution and would like to be able
to use something akin to it.

So long as you don't try patenting it (not that I claim to be the first to
'discover' it). ;-)
Thank you again for your help.

No problem. Thanks for turning the topic back to standard C++.

I don't want to seem ingrateful but I am having trouble making your code
compile. It has several small bugs, and working with it piece-meal is very
challenging. I am also having significant doubts as to whether there are any
advantages of your approach with regards to the most recent code I posted. I
would appreciate either a full compilable version that I may work with or a
brief explanation of why I would want to try and continue to with the code
you have given me over the code I have posted. I have made my code available
within the context of some tests at
http://www.heron-language.com/cpp-iop-example.html if you want to look it
over.

The technique you used, where did it come from? Someone else on comp.std.c++
posted something similar, and I am curious as to its origins.

Thanks again,
 
C

Claudio Puviani

christopher diggins said:
I don't want to seem ingrateful but I am having trouble
making your code compile. It has several small bugs,
and working with it piece-meal is very challenging.

I wrote it inline of a posting, as I said, as an example. I'm sure there are a
number of little imperfections (like I forgot to return *this from operator=).
I am also having significant doubts as to whether there
are any advantages of your approach with regards to
the most recent code I posted.

It's an alternavive design that uses built-in C++ features instead of
duplicating them. There's no obligation for you to use it. If it gives you a
few ideas, great. If not, feel free to ignore it.
I would appreciate either a full compilable version that I
may work with

I'll look into it, but I have other things that need to take priority. Also,
keep in mind that some older compilers might not handle template member
functions.
or a brief explanation of why I would want to try and
continue to with the code you have given me over the
code I have posted.

As I said above, whether or not you use it is entirely up to your personal
preferences. I'm not trying to persuade you to change. There's nothing
inherently wrong with your approach. I just offered an alternative approach.
I have made my code available within the context of some
tests at http://www.heron-language.com/cpp-iop-example.html
if you want to look it over.

Now this, I definitely won't have time to look into.
The technique you used, where did it come from?

The particular implementation was just off the top of my head.
Someone else on comp.std.c++ posted something similar,
and I am curious as to its origins.

This is just a variant of the old Proxy and Facade patterns, so you're likely
to see a lot of solutions that are similar to yours or mine, and a few that
neither of us ever saw but which would look obvious as soon as we did see them.
The origins predate C++, but the pattern is published in almost every pattern
book and different solutions appear in various C++ books.

Claudio Puviani
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,769
Messages
2,569,580
Members
45,054
Latest member
TrimKetoBoost

Latest Threads

Top