views:

230

answers:

4

This is a poll for opinions on the most readable way to do something -- whether to use a C++ pointer-to-member, a byte offset, or a templatized functor to define "select member X from structure foo".

I've got a type that contains a large vector of structures, and I'm writing a utility function that basically operates as a reduce over some range of them. Each structure associates a group of dependent variables with some point along an independent dimension -- to invent a simplified example, imagine that this records a series of environmental conditions for a room over time:

// all examples are psuedocode for brevity
struct TricorderReadings
{
  float time;  // independent variable

  float tempurature;
  float lightlevel;
  float windspeed; 
  // etc for about twenty other kinds of data...
}

My function simply performs a cubic interpolation to guess those conditions for some given point of time in between the available samples.

// performs Hermite interpolation between the four samples closest to given time
float TempuratureAtTime( float time, sorted_vector<TricorderReadings> &data)
{
    // assume all the proper bounds checking, etc. is in place
    int idx = FindClosestSampleBefore( time, data );
    return CubicInterp( time, 
                        data[idx-1].time, data[idx-1].tempurature,
                        data[idx+0].time, data[idx+0].tempurature,
                        data[idx+1].time, data[idx+1].tempurature,
                        data[idx+2].time, data[idx+2].tempurature );
}

I'd like to generalize this function so that it can be applied generically to any member, not just temperature. I can think of three ways to do this, and while they're all straightforward to code, I'm not sure what will be the most readable to whoever has to use this a year from now. Here's what I'm considering:


Pointer-to-member syntax

typedef int TricorderReadings::* selector;
float ReadingAtTime( time, svec<TricorderReadings> &data, selector whichmember )
{
   int idx = FindClosestSampleBefore( time, data );
   return CubicInterp( time, data[idx-1].time, data[idx-1].*whichmember, 
                       /* ...etc */  );
}
// called like:
ReadingAtTime( 12.6f, data, &TricorderReadings::windspeed );

This feels like the most "C++y" way to do it, but it looks strange, and the whole pointer-to-member syntax is rarely used and therefore poorly understood by most people on my team. It's the technically "right" way, but also the one I'll get the most confused emails about.

Structure offset

float ReadingAtTime( time, svec<TricorderReadings> &data, int memberoffset )
{
   int idx = FindClosestSampleBefore( time, data );
   return CubicInterp( time, 
                       data[idx-1].time, 
                       *(float *) ( ((char *)(&data[idx-1]))+memberoffset ), 
                       /* ...etc */  );
}
// called like:
ReadingAtTime( 12.6f, data, offsetof(TricorderReadings, windspeed) );

This is functionally identical to the above, but does the pointer math explicitly. This approach will be immediately familiar and understandable to everyone on my team (who all learned C before C++), and it is robust, but it just seems icky.

Templatized Functor

template <class F>
float ReadingAtTime( time, svec<TricorderReadings> &data )
{
   int idx = FindClosestSampleBefore( time, data );
   return CubicInterp( time, 
                       data[idx-1].time, 
                       F::Get(data[idx-1]) ), 
                       /* ...etc */  );
}

// called with:
class WindSelector
{ 
   inline static float Get(const TricorderReadings &d) { return d.windspeed; }
}
ReadingAtTime<WindSelector>( 12.6f, data );

This is the most straightforward and STL-ish way of doing things, but it seems like a whole bunch of extra typing and syntax and extemporaneous class definitions. It compiles to almost exactly the same thing as the two above, but it also dumps a bunch of redundant function definitions all over the executable. (I've verified this with /FAcs, but maybe the linker takes them out again.)


All the three above will work, and the compiler emits pretty much the same code for all of them; so, the most important choice I have to make is simply which is most readable. What do you think?

+3  A: 

I find the Templatized Functor very clear in this case.

ReadingAtTime<WindSelector>( 12.6f, data );
AraK
+1  A: 

If your team is composed of reasonably smart people, I would say to trust them and their abilities, and use the technically preferred solution that pointer-to-member syntax offers. This is the kind of thing it was made for.

If you're really concerned, you can take some steps to ease any future trouble by

  • Noting in a comment near the typedef and the usage that this is called "pointer to member" syntax, so that other team members know what to look up
  • Point it out explicitly in a code review, where many of them should be present. Offer to change it if it's considered incomprehensible or too obscure for maintenance.

The other two approaches have problems, both as you described, and beyond:

  • Both require more code, have more room for typos, etc.
  • The offsetof primitive is limited in what types it can be applied to:

    Because of the extended functionality of structs in C++, in this language, the use of offsetof is restricted to "POD types", which for classes, more or less corresponds to the C concept of struct (although non-derived classes with only public non-virtual member functions and with no constructor and/or destructor would also qualify as POD).

From here.

Novelocrat
I think I'll go with this one. You make the good point that any developer worth his paycheck ought to be able to learn a bit of syntax by reading a comment; and I think I'd face an armed rebellion from the team if I obliged them to construct a tiny class on the fly every time they wanted to interpolate a variable.
Crashworks
+1  A: 

For simple stuff, I'd prefer the Pointer-to-member solution. However, there are two possible advantages to the functor approach:

  1. separating the algorithm from the data allows you to use the algorithm for more things in the future, sine it work with anything provided you can construct a proper functor.

  2. related to #1, this might make testing the algorithm easier, as you have a way to provide test data to the function that does not involve creating the full data objects you intend to use. You can use simpler mock objects.

However, I think the functor approach is worth it only if the function you're making is very complex and/or used in many different places.

MadCoder
+2  A: 

A more STL-ish way would be a generic functor that makes access through a pointer-to-member look like a function call. It might look something like this:

#include <functional>

template <class T, class Result>
class member_pointer_t: public std::unary_function<T, Result>
{
    Result T::*member;
public:
    member_pointer_t(Result T::*m): member(m) {}
    Result operator()(const T& o) const { return o.*member; }
};

template <class T, class Result>
member_pointer_t<T, Result> member_pointer(Result T::*member)
{
    return member_pointer_t<T, Result>(member);
}

float ReadingAtTime( float time, const std::vector<TricorderReadings> &data, member_pointer_t<TricorderReadings, float> f )
{
   int idx = FindClosestSampleBefore( time, data );
   return CubicInterp( time, data[idx-1].time, f(data[idx-1]));
}

ReadingAtTime( 12.6f, data, &TricorderReadings::windspeed);

The example also includes a helper function to help deduce template arguments for the functor (not used in this example).

The function ReadingAtTime might also accept a templated functor:

template <class Func>
float ReadingAtTime( float time, const std::vector<TricorderReadings>& data, Func f);

ReadingAtTime( 12.6f, data, member_pointer(&TricorderReadings::windspeed));

This way you could use all kinds of functions / functors to obtain a value from data[idx - 1], not just pointers-to-member.

More generic equivalents of member_pointer might be std::tr1::bind or std::tr1::mem_fn.

UncleBens