How to write a generic "getData" function?

Question

Hi, everyone!

I have a class, say, "CDownloader", that reads some XML data and provides access by node names. It features some getter functions, something like this:

BOOL CDownloader::getInteger ( const CString &name, int *Value );
BOOL CDownloader::getImage   ( const CString &name, BOOL NeedCache, CImage *Image );
BOOL CDownloader::getFont    ( const CString &name, CFont *Font );

I cannot change CDownloader class. Instead I would like to write some functions, that downloads items by using a bool flag, not an actual name. Something like this:

BOOL DownloadFont( const CDownloader &Loader, bool Flag, CFont *Font )
{
   if (Flag) {
      // first try the "name_1"
      if ( Loader.getFont("name_1", Font) ) return TRUE;
   }
   // if "name_1" fails or disabled by flag, try "name_2"
   return Loader.getFont("name_2", Font);
}

I can write Download(Font|Integer|Image) functions separatly, but this will result in code duplication. My idea is to write a template, but I am still at a loss: how can I determine what method should I call from CDownloader class? To specialize template for each data type means to stuck into code duplication again. To pass getter funciton as a "pointer-to-function" parameter? But the getter signatures differ in CDownloader...

Summing it up, the question is: is it possible to write a generic wrapper around CDownloader or do I have to duplicate code for each "get***" function? Thanks in advance!

Answer 1

Maybe using a delegate ?

Answer 2

I don't think writing a generic wrapper can end up being less code/duplication due to the fact that the method signatures for the 3 getters are different. You'll need wrapper functions around these no matter what. You might as well go with the straightforward way of having 3 different Download* functions. You could use macros to keep the conditional logic in a central location, but that would probably make your code grossly unreadable, and it's not worth it.

Answer 3

As long as you have three differently named functions and need to pick one depending on the type, at some point you have to have either an overload or some traits class to picks the right one. I don't think there's a way around that. However, since the call to one of these function is the only thing that needs this, if there is more code to these DownloadXXX() functions than you showed us, then it might still make sense.

Here's a sketch of what you might do using the overload alternative. First you need three overloads of the same function each calling one of the three different functions. The additional BOOL parameter for one of the functions somewhat wreaks havoc with the genericity, but I got around that by having all functions accept that BOOL, but two of them ignoring it:

inline BOOL Load(CDownloader& Loader, const CString &name, int &Value, BOOL)
{return Loader.getInteger(name, &Value);

inline BOOL Load(CDownloader& Loader, const CString &name, CImage &Value, BOOL NeedCache)
{return Loader.getImage(name, NeedCache, &value);

inline BOOL Load(CDownloader& Loader, const CString &name, CFont &Value, BOOL)
{return Loader.getFont(name, &Font);

Now you can go and write that generic function. You need to decide what to do about that BOOL, though:

template< typename T >
BOOL Download(const CDownloader &Loader, bool Flag, T &Obj, BOOL NeedCache /*= true*/)
{
   if (Flag) {
      if ( Load(Loader, "name_1", Obj, NeedCache) ) return TRUE;
   }
   return Load(Loader, "name_1", Obj, NeedCache);
}

However, as you can see, this is really only worth the hassle if that Download function is a lot more complicated than in your sample code. Otherwise the added complexity easily outweighs the gains that the increased genericity brings.

Answer 4

As Ates writes in his answer, you still have to write wrappers for the CDownloader members, so the end result is likely to be as verbose and harder to understand than the straightforward way. For example, this could be a possibility (warning: untested code ahead):

BOOL Get(const CDownloader &Loader, const CString& Name, int* Result)
{
    return Loader.getInteger(Name, Result);
}

BOOL Get(const CDownloader &Loader, const CString& Name, CImage* Result)
{
    return Loader.getImage(Name, SomeDefaultValueForNeedCache, Result);
}

BOOL Get(const CDownloader &Loader, const CString& Name, CFont* Result)
{
    return Loader.getFont(Name, Result);
}


template<class T>
BOOL Download(const CDownloader &Loader, bool Flag, T* Result)
{
   if (Flag) {
      // first try the "name_1"
      if ( Get(Loader, "name_1", Result) ) return TRUE;
   }
   // if "name_1" fails or disabled by flag, try "name_2"
   return Get (Loader, "name_2", Result);
}

Trying to be "cleverer", one could try to make a boost::fusion::map of the getters, indexed by the "getted" type:

fusion::map<
    fusion::pair<int, boost::function<BOOL(const CDownloader&, int*)>,
    fusion::pair<CImage, boost::function<BOOL(const CDownloader&, CImage*)>,
    fusion::pair<CFont, boost::function<BOOL(const CDownloader&, CFont*)>
>
GetterMap = fusion::make_map(
    fusion::make_pair<int>(bind(&CDownloader::getInteger, _1, _2)), 
    fusion::make_pair<CImage>(&CDownloader::getImage, _1, SomeDefaultValueForNeedCache, _2),
    fusion::make_pair<CFont>(&CDownloader::getFont, _1, _2)
);


template<class T>
BOOL Download(const CDownloader &Loader, bool Flag, T* Result)
{
   if (Flag) {
      // first try the "name_1"
      if ( fusion::at<T>(GetterMap)(Loader, "name_1", Result) ) return TRUE;
   }
   // if "name_1" fails or disabled by flag, try "name_2"
   return fusion::at<T>(GetterMap)(Loader, "name_2", Result);
}

As you see, the gain compared to the straightforward way is not obvious.

Answer 5

You can get somewhere with a pointer to member function:

struct X
{
    bool getInt(int* p) const { *p = 42; return true; }
    bool getFloat(float* p) const { *p = 3.14; return true; }
};

template <class Func, class T>
bool load(const X& x, Func f, T* t)
{
    return (x.*f)(t);
}

int main()
{
    int i;
    float f;
    X x;
    load(x, &X::getInt, &i);
    load(x, &X::getFloat, &f);

    //load(x, &X::getFloat, &i);
}

Now the exception of the getImage method makes it harder. May-be try making this work with something like boost::bind / std::tr1::bind instances instead.

#include <boost/bind.hpp>

struct X
{
    bool getInt(int* p) const { *p = 42; return true; }
    bool getFloat(float* p, bool b) const { *p = 3.14; return b; }
};

template <class Func, class T>
bool load(Func f, T* t)
{
    return f(t);
}

int main()
{
    using namespace boost;
    int i;
    float f;
    X x;
    load(bind(&X::getInt, x, _1), &i);
    load(bind(&X::getFloat, x, _1, true), &f);
}

Answer 6

Here's a C-hacky way to do it.

void* DownloadFont( const CDownloader &Loader, bool Flag, CFont *Font )
{
   if (Flag) {
      // first try the "name_1"
      if ( Loader.getFont("name_1", Font) ) return (void*)1; //access this directly and *die*
   }
   // if "name_1" fails or disabled by flag, try "name_2"
   return (void*)(Loader.getFont("name_2", Font);
}

In the end, you're going to need a logic somehow that relates to specialized getting of integer/font/image/foobars/magic monkeys. I'd just suck it up and write a Download*() family.

Answer 7

I think function objects are best because you can adapt to different signatures.

struct FontLoader {
    CFont *Font;
    FontLoader() {}
    BOOL operator()(const CDownloader& Loader, bool Flag) {
        if (Flag && Loader.getFont("name_1", Font) ) 
            return TRUE;
        return Loader.getFont("name_2", Font);
    }
};

struct ImageLoader {
    CImage *Image;
    BOOL NeedCache;
    ImageLoader(BOOL nc) : NeedCache(nc) {}
    BOOL operator()(const CDownloader& Loader, bool Flag) {
        if (Flag && Loader.getImage("name_3", NeedCache, Image) ) 
            return TRUE;
        return Loader.getImage("name_4", NeedCache, Image);
    }          
};

template <typename T> // T has application operator CDownloader x bool -> T1
BOOL Download( const CDownloader &Loader, bool Flag, T& func)
{
    return func(Loader, Flag);
}

The calls would then look like:

FontLoader Font_func;
BOOL ret1 = Download(Loader, Flag, Font_func);
ImageLoader Image_func(TRUE);
BOOL ret2 = Download(Loader, Flag, Image_func);

and the structs passed in would contain the downloaded objects. In C++0x you will be able to define a concept that will provide better type checking on the template parameter T.