views:

336

answers:

12

The problem:

I have a C++ class with gajillion (>100) members that behave nearly identically:

  • same type

  • in a function, each member has the same exact code done to it as other members, e.g. assignment from a map in a constructor where map key is same as member key

  • This identicality of behavior is repeated across many-many functions (>20), of course the behavior in each function is different so there's no way to factor things out.

  • The list of members is very fluid, with constant additions and sometimes deletions, some (but not all) driven by changing columns in a DB table.

As you can imagine, this presents a big pain-in-the-behind as far as code creation and maintenance, since to add a new member you have to add code to every function where analogous members are used.

Example of a solution I'd like

Actual C++ code I need (say, in constructor):

MyClass::MyClass(SomeMap & map) { // construct an object from a map 
    intMember1 = map["intMember1"];
    intMember2 = map["intMember2"];
    ... // Up to 
    intMemberN = map["intMemberN"];
}

C++ code I want to be able to write:

MyClass::MyClass(SomeMap & map) { // construct an object from a map 
#FOR_EACH_WORD Label ("intMember1", "intMember2", ... "intMemberN")
    $Label = map["$Label"];
#END_FOR_EACH_WORD
}

Requirements

  • The solution must be compatible with GCC (with Nmake as make system, if that matters). Don't care about other compilers.

  • The solution can be on a pre-processor level, or something compilable. I'm fine with either one; but so far, all of my research pointed me to the conclusion that the latter is just plain out impossible in C++ (I so miss Perl now that I'm forced to do C++!!!)

  • The solution must be to at least some extent "industry standard" (e.g. Boost is great, but a custom Perl script that Joe-Quick-Fingers created once and posted on his blog is not. Heck, I can easily write that Perl script, being much more of a Perl expert than a C++ one - I just can't get bigwigs in Software Engineering at my BigCompany to buy into using it :) )

  • The solution should allow me to declare a list of IDs (ideally, in only one header file instead of in every "#FOR_EACH_WORD" directive as I did in the example above)

  • The solution must not be limited to "create an object from a DB table" constructor. There are many functions, most of them not constructors, that need this.

  • A solution of "Make them all values in a single vector, and then run a 'for' loop across the vector" is an obvious one, and can not be used - the code's in a library used by many apps, the members are public, and re-writing those apps to use vector members instead of named members is out of the question, sadly.

A: 

You can do something like his:

#define DOTHAT(m) m = map[#m]
DOTHAT(member1); DOTHAT(member2);
#undef DOTHAT

That doesn't fully fit your description, but closest to it that saves you typing.

Michael Krelin - hacker
+3  A: 

You could do something like this: create an adapter class or modify the existing class to have a vector of pointers to those fields, add the addresses of all member variables in question to that vector in the class constructor, then when needed run the for-loop on that vector. This way you don't (or almost don't) change the class for external users and have a nice for-loop capability.

sharptooth
I really like this idea, although the "almost" part is what makes it un-usable here. the mandate excludes ANY modifications to apps using this class. Wonder if I can turn the current class into the adapter class?
DVK
Technically you can, but you'll have to recompile the consumer applications unless you only show them the interfaces.
sharptooth
Having a vector with pointers to the members doesn't seem to require to change the class where those members are stored. Thus this looks like the solution you wanted to have. Why would you need to change the class for external users?
Johannes Schaub - litb
+2  A: 

You could use the preprocessor to define the members, and later use the same definition to access them:

#define MEMBERS\
  MEMBER( int, value )\
  SEP MEMBER( double, value2 )\
  SEP MEMBER( std::string, value3 )\

struct FluctuatingMembers {
#define SEP ;
#define MEMBER( type, name ) type name
MEMBERS
#undef MEMBER
#undef SEP
};


.. client code:
FluctuatingMembers f = { 1,2., "valuesofstringtype" };
std::cout <<
  #define SEP <<
  #define MEMBER( type, name ) #name << ":" << f.##name
  MEMBERS;
  #undef MEMBER
  #undef SEP

It worked for me, but is hard to debug.

xtofl
A variant is to remplace MEMBERS by an include (which can contains the #undefs).
AProgrammer
good tip to add the undefs there... Although it breaks the symmetry somehow. heck, it's an ugly solution anyway :)
xtofl
+2  A: 

Surreptitiously use perl on your own machine to create the constructor. Then ask to increase your salary since you're succesfully maintaining such a huge chunk of code.

Pavel Shved
Actually, even if you meant this as a joke, this is a perfect way of solving this problem. A simple text file could be written that lists all the members of the table (this could even be generated by dumping the DB schema). Then the perl script would generate all the necessary code. (+1).
Richard Corden
You could do the same thing with the c preprocessor.
Alan
I didn't make it a joke. Totally no. If the bosses so narrow, that they forbid perl, this is the perfect way to both do the job without menial work and get the credit.
Pavel Shved
+2  A: 

You can also implement a visitor pattern based on pointer-to-members. After the preprocessor solution, this one turns out way more debuggeable.

struct FluctuatingMembers {
    int v1;
    double v2;
    std::string v3;
    template<typename Visitor> static void each_member( Visitor& v );
};

template<typename Visitor> void FluctuatingMembers::each_member( Visitor& v ) {
  v.accept( &FluctuatingMembers::v1 );
  v.accept( &FluctuatingMembers::v2 );
  v.accept( &FluctuatingMembers::v3 );
}


struct Printer {
    FluctuatingMembers& f;
    template< typename pt_member > void accept( pt_member m ) const {
        std::cout << (f::*m) << "\n";
    }
};

// you can even use this approach for visiting
// multiple objects simultaneously
struct MemberComparer {

    FluctuatingMembers& f1, &f2;
    bool different;
    MemberComparer( FluctuatingMembers& f1, FluctuatingMembers& f2 )
      : f1(f1),f2(f2)
      ,different(false)
    {}

    template< typename pt_member > void accept( pt_member m ) {
      if( (f1::*m) != (f2::*m) ) different = true;          
    }
};

... client code:
FluctuatingMembers object1 = { 1, 2.2, "value2" }
                 , object2 = { 1, 2.2, "valuetoo" };

Comparer compare( object1, object2 );
FluctuatingMembers::each_member( compare );
Printer pr = { object1 };
FluctuatingMembers::each_member( pr );
xtofl
You combined the essence of a really nice response with a lot of noise. Pointer to member syntax provides the necessary abstraction, but tying it to the Visitor pattern is a distraction when a different overall construct might work better. For some other discussion of pointer-to-member syntax, have a look at http://stackoverflow.com/questions/1344840
Novelocrat
This answer seems to be overly complex: The members all have the same type, so a visitor pattern seems to be overkill. I may have misunderstood something though.
Johannes Schaub - litb
@litb: that's right. But if the members fluctuate a lot, sooner or later they'll become doubles and floats too, in which case this approach is general enough.
xtofl
+3  A: 

Of course, the obvious question is: Why do you have a class with 100 members? It doesn't really seem sane.

Assuming it is sane nevertheless -- have you looked at boost preprocessor library? I have never used it myself (as one friend used to say: doing so leads to the dark side), but from what I heard it should be the tool for the job.

sbi
The answer is: This is a class from a legacy application suite; and re-factoring the whole bloody thing is not on the list of things likely to be approved to spend time on.
DVK
BTW, I'm not 100% certain this class CAN be designed with less than 100 members since as I mentioned, it's largely based on a DB table with 100s of columns. Which are not normalizable to the bets of my analysis (and not owned by my team in the first place :)
DVK
+8  A: 

Boost includes a great preprocessor library that you can use to generate such code:

#include <boost/preprocessor/repetition.hpp>
#include <boost/preprocessor/stringize.hpp>
#include <boost/preprocessor/cat.hpp>

typedef std::map<std::string, int> SomeMap;

class MyClass
{
public:
    int intMember1, intMember2, intMember3;

    MyClass(SomeMap & map) 
    {
        #define ASSIGN(z,n,_) BOOST_PP_CAT(intMember, n) = map[ BOOST_PP_STRINGIZE(BOOST_PP_CAT(intMember, n))];
        BOOST_PP_REPEAT_FROM_TO(1, 4, ASSIGN, nil)
    }
};
Bojan Resnik
We might suppose that class members have more meaningful names. Therefore, it might not be possible to use a simple for loop. I'll suggest using a Boost.PP sequence, along with the for_each algorithm.
Luc Touraille
I just saw that it was an explicit requirement of the OP: "The solution should allow me to declare a list of IDs". I posted an answer showing how to do this, feel free to add it to your own in order to have a more complete answer.
Luc Touraille
You are absolutely correct - I was responding to the OP's claim that the given code was "*actual C++ code*" he needed.
Bojan Resnik
A: 

Probably what I'd look to do would be to make use of runtime polymorphism (dynamic dispatch). Make a parent class for those members with a method that does the common stuff. The members derive their class from that parent class. The ones that need a different implementation of the method implement their own. If they need the common stuff done too, then inside the method they can downcast to the base class and call its version of the method.

Then all you have to do inside your original class is call the member for each method.

T.E.D.
A: 

I would recommend a small command-line app, written in whatever language you or your team are most proficient in.

Add some kind of template language to your source files. For something like this, you don't need to implement a full-fledged parser or anything fancy like that. Just look for an easily-identified character at the beginning of a line, and some keywords to replace.

Use the command-line app to convert the templated source files into real source files. In most build systems, this should be pretty easy to do automatically by adding a build phase, or simply telling the build system: "use MyParser.exe to handle files of type *.tmp"

Here's an example of what I'm talking about:

MyClass.tmp

MyClass::MyClass(SomeMap & map) { // construct an object from a map
▐REPLACE_EACH, LABEL, "intMember1", "intMember2, ... , "intMemberN"
▐   LABEL = map["$Label"];
}

I've used "▐" as an example, but any character that would otherwise never appear as the first character on a line is perfectly acceptable.

Now, you would treat these .tmp files as your source files, and have the actual C++ code generated automatically.

If you've ever heard the phrase "write code that writes code", this is what it means :)

e.James
Sorry, but as I said in the question's requirements, (1) I can easily write my perl preprocessed, but (2) the powers that be would never approve of using a custom thing like that in company wide build system. In case it was not epxlicitly obvious, having my own setup that pre-processes prior to CVS check-ins is out of the question since it is shared code maintained by many developers.
DVK
You write the Perl preprocessor. Then you run it, and get chunks of code, which you copy-paste into the C++ program. It's a method of saving you writing code up front, rather than making this Codezilla fundamentally maintainable (which you are forbidden to do).
David Thornley
@DVK: I am sorry. I missed that requirement in your question. My fault for reading through it too quickly. I think David Thornley has the right idea, though. Even if you can't get the whole company to use the utility, you can still use it when you have to maintain the code.
e.James
Either way, it sounds like the project is being held back by an overall reluctance to implement change. Perhaps it is time to start working on your resume? :)
e.James
Actually, it's overall in large picture a great place to work. - Developers are not merely encouraged, but are actually forced to learn business- Developers are (and are treated as) a resource instead of a cost center.- Despite somewhat overburdening bueraocracy (sp?) and red tape, the amount of both is SIGNIFICANLY less (and application of those is SIGNIFICATLY more intelligent) than in many other large companies.
DVK
+5  A: 

Boost.Preprocessor proposes many convenient macros to perform such operations. Bojan Resnik already provided a solution using this library, but it assumes that every member name is constructed the same way.

Since you explicitely required the possibily to declare a list of IDs, here is a solution that should better fulfill your needs.

#include <boost/preprocessor/seq/for_each.hpp>
#include <boost/preprocessor/stringize.hpp>

// sequence of member names (can be declared in a separate header file)
#define MEMBERS (foo)(bar)

// macro for the map example
#define GET_FROM_MAP(r, map, member) member = map[BOOST_PP_STRINGIZE(member)];

BOOST_PP_SEQ_FOR_EACH(GET_FROM_MAP, mymap, MEMBERS)
// generates
// foo = mymap["foo"]; bar = mymap["bar];

-------

//Somewhere else, we need to print all the values on the standard output:
#define PRINT(r, ostream, member) ostream << member << std::endl;

BOOST_PP_SEQ_FOR_EACH(PRINT, std::cout, MEMBERS)

As you can see, you just need to write a macro representing the pattern you want to repeat, and pass it to the BOOST_PP_SEQ_FOR_EACH macro.

Luc Touraille
Question - how well does this Boost wizardry work with GDB? It may not be a deciding factor, bit I would hate to have lost ability to debug properly.
DVK
I never had to debug a program using Boost.PP with GDB, so I can't properly answer this question. However, I know that the Visual Studio debugger step over the macro without entering it...A solution to that could be to compile and debug the preprocessed file (see gcc -E, or cl.exe /P). You should note though that BOOST_PP_SEQ_FOR_EACH is an horizontal repetition macro: everything it generates will be on the same line. You can check vertical repetition, which may facilitate debugging.
Luc Touraille
+1  A: 

Why not do it at run time? (I really hate macro hackery)

What you really are asking for, in some sense, is class metadata.

So I would try something like:

class AMember{
 ......
};

class YourClass{
    AMember member1;
    AMember member2;
    ....
    AMember memberN;
    typedef AMember YourClass::* pMember_t;
    struct MetaData : public std::vector<std::pair<std::string,pMember_t>>{
        MetaData(){
            push_back(std::make_pair(std::string("member1"),&YourClass::member1));
            ...
            push_back(std::make_pair(std::string("memberN"),&YourClass::memberN)); 
        }
    };

    static const MetaData& myMetaData() {
        static const MetaData m;//initialized once
        return m;
    }

    YourClass(const std::map<std::string,AMember>& m){
        const MetaData& md = myMetaData();
        for(MetaData::const_iterator i = md.begin();i!= md.end();++i){
            this->*(i->second) = m[i->first];
        }
    }
    YourClass(const std::vector<std::pair<std::string,pMember_t>>& m){
        const MetaData& md = myMetaData();
        for(MetaData::const_iterator i = md.begin();i!= md.end();++i){
            this->*(i->second) = m[i->first];
        }
    }
};

(pretty sure I've got the syntax right but this is a machinery post not a code post)

RE: in a function, each member has the same exact code done to it as other members, e.g. assignment from a map in a constructor where map key is same as member key

this is handled above.

RE: The list of members is very fluid, with constant additions and sometimes deletions, some (but not all) driven by changing columns in a DB table.

When you add a new AMember, say newMember, all you have to do is update the MetaData constructor with an:

 push_back(make_pair(std::string("newMember"),&YourClass::newMember));

RE: This identicality of behavior is repeated across many-many functions (>20), of course the behavior in each function is different so there's no way to factor things out.

You have the machinery to apply this same idiom to build the functions

eg: setAllValuesTo(const AMember& value)

 YourClass::setAllValuesTo(const AMember& value){
    const MetaData& md = myMetaData();
    for(MetaData::const_iterator i = md.begin();i!= md.end();++i){
        this->*(i->second) = value;
    }
 }

If you are a tiny bit creative with function pointers or template functionals you can factor out the mutating operation and do just about anything you want to YourClass' AMember's on a collection basis. Wrap these general functions (that may take a functional or function pointer) to implement your current set of 20 public methods in the interface.

If you need more metadata just augment the codomain of the MetaData map beyond a pointer to member. (Of course the i->second above would change then)

Hope this helps.

pgast
A: 

There are already a lot of good answers and ideas here, but for the sake of diversity I'll present another.

In the code file for MyClass would be:

struct MemberData
{
    size_t Offset;
    const char* ID;
};

static const MemberData MyClassMembers[] = 
{
    { offsetof(MyClass, Member1), "Member1" },
    { offsetof(MyClass, Member2), "Member2" },
    { offsetof(MyClass, Member3), "Member3" },
};

size_t GetMemberCount(void)
{
    return sizeof(MyClassMembers)/sizeof(MyClassMembers[0]);
}

const char* GetMemberID(size_t i)
{
    return MyClassMembers[i].ID;
}

int* GetMemberPtr(MyClass* p, size_t i) const
{
    return (int*)(((char*)p) + MyClassMembers[i].Offset);
}

Which then makes it possible to write the desired constructor as:

MyClass::MyClass(SomeMap& Map)
{
    for(size_t i=0; i<GetMemberCount(); ++i)
    {
        *GetMemberPtr(i) = Map[GetMemberID(i)];
    }
}

And of course, for any other functions operating on all the members you would write similar loops.

Now there are a few issues with this technique:

  • Operations on members use a runtime loop as opposed to other solutions which would yield an unrolled sequence of operations.
  • This absolutely depends on each member having the same type. While that was allowed by OP, one should still evaluate whether or not that might change in the future. Some of the other solutions don't have this restriction.
  • If I remember correctly, offsetof is only defined to work on POD types by the C++ standard. In practice, I've never seen it fail. However I haven't used all the C++ compilers out there. In particular, I've never used GCC. So you would need to test this in your environment to ensure it actually works as intended.

Whether or not any of these are problems is something you'll have to evaluate against your own situation.


Now, assuming this technique is usable, there is one nice advantage. Those GetMemberX functions can be turned into public static/member functions of your class, thus providing this generic member access to more places in your code.

class MyClass
{
public:
    MyClass(SomeMap& Map);

    int Member1;
    int Member2;
    int Member3;

    static size_t GetMemberCount(void);
    static const char* GetMemberID(size_t i);
    int* GetMemberPtr(size_t i) const;
};

And if useful, you could also add a GetMemberPtrByID function to search for a given string ID and return a pointer to the corresponding member.


One disadvantage with this idea so far is that there is a risk that a member could be added to the class but not to the MyClassMembers array. However, this technique could be combined with xtofl's macro solution so that a single list could populate both the class and the array.

changes in the header:

#define MEMBERS\
    MEMBER( Member1 )\
    SEP MEMBER( Member2 )\
    SEP MEMBER( Member3 )\

class MyClass
{
public:
    #define SEP ;
    #define MEMBER( name ) int name
    MEMBERS;
    #undef MEMBER
    #undef SEP

    // other stuff, member functions, etc
};

and changes in the code file:

const MemberData MyClassMembers[] = 
{
    #define SEP ,
    #define MEMBER( name ) { offsetof(MyClass, name), #name }
    MEMBERS
    #undef MEMBER
    #undef SEP
};


Note: I have left error checking out of my examples here. Depending on how this would be used, you might want to ensure the array bounds are not overrun with debug mode asserts and/or release mode checks that would return NULL pointers for bad indexes. Or some use of exceptions if appropriate.

Of course, if you aren't worried about error checking the array bounds, then GetMemberPtr could actually be changed into something else that would return a reference to the member.

TheUndeadFish