Changing a fragment of a specific function into a test function, how to pass an argument?
I am trying to refactor some code to make it testable. Most of it uses #define to fill in duplicate code in functions. I am trying to change it in real functions, but I ran into a stumbling block.
Note: source code from anti-aliasing algorithm
typedef struct{
unsigned char R, G, B;
} RGBTriple;
#define compute_disperse(channel) \
error = ((int)(currentPixel->channel)) - palette.table[index].channel; \
image.pixels[(x+1) + (y+0)*image.width].channel += (error*7) >> 4; \
image.pixels[(x-1) + (y+1)*image.width].channel += (error*3) >> 4; \
image.pixels[(x+0) + (y+1)*image.width].channel += (error*5) >> 4; \
image.pixels[(x+1) + (y+1)*image.width].channel += (error*1) >> 4;
inside the actual method
RGBTriple* currentPixel = &(image.pixels[x + y*image.width]);
compute_disperse(R);
compute_disperse(G);
compute_disperse(B);
If I try to convert compute_disperse to a valid function, then I need to somehow pass the "channel" as R, G or B values.
I tried to pass it as unsigned char but I am getting error
error: 'struct RGBTriple' has no member named 'channel'
I've read about the definition of RG and B, but can't get it to work ... How can I convert the given section to an actual function instead of a macro?
source to share
In response to Slava's answer, you can use member pointers ... as template arguments!
template <unsigned char RGBTriple::*Tchannel>
compute_disperse(/* Whatever you need */) {
error = ((int)(currentPixel->*Tchannel)) - palette.table[index].*Tchannel;
/* ... */
}
Least typing, flexible and fully built in by any decent compiler.
source to share
The obvious solution would be to pass a pointer to the member unsigned char RGBTriple::*channel
, but I'm sure it would have performance issues. I would use the template instead:
enum ChannelName { R, G, B };
template <ChannelName>
unsigned char &getChannel( RGBTriple &rgb );
template <>
unsigned char &getChannel<R>( RGBTriple &rgb ) { return rgb.R; }
template <>
unsigned char &getChannel<G>( RGBTriple &rgb ) { return rgb.G; }
template <>
unsigned char &getChannel<B>( RGBTriple &rgb ) { return rgb.B; }
then
template <ChannelName ch>
compute_disperse( /* parameters, probably currentPixel, pallette and image */ ) {
error = ((int)getChannel<ch>( *currentPixel ) - getChannel<ch>( palette.table[index] );
getChannel<ch>( image.pixels[(x+1) + (y+0)*image.width] ) += (error*7) >> 4;
...
}
then you will call compute_disperse
like this:
compute_disperse<R>( /* parameters */ );
compute_disperse<G>( /* parameters */ );
compute_disperse<B>( /* parameters */ );
What parameters are required to provide a function are beyond the scope of this answer, you need to analyze what data will be available to this function.
source to share
Use association for your type RGB, allowing you to refer to channels like R
, G
, B
and as an array. Then write an enum with pipes (actually used as array indices):
struct RGB
{
union
{
struct { unsigned char R , G , B };
unsigned char channels[3];
};
};
enum rgb_channel{ R = 0 , G = 1 , B = 2 };
Here's an example:
unsigned char get_channel_value( RBG color , rgb_channel channel )
{
return color.channels[(int)channel]; //The cast is not strictly necessary, but
//makes the code more concise.
}
//You could do this:
color.R = 0;
//Or this:
color.channels[R] = 0;
source to share
This hasn't changed:
typedef struct{
unsigned char R, G, B;
} RGBTriple;
Typedef: accessor function:
typedef char & Accessor(RGBTriple *);
Define access functions for each channel
char & AccessR(RGBTriple * pixel) { return pixel->R;}
char & AccessG(RGBTriple * pixel) { return pixel->G;}
char & AccessB(RGBTriple * pixel) { return pixel->B;}
Here's your new compute_disperse function (not a macro)
void compute_disperse(RGBTriple * currentPixel, Accessor access)
{
int error = ((int)access(currentPixel) - access(palette.table[index]);
access(image.pixels[(x+1) + (y+0)*image.width]) += (error*7) >> 4;
access(image.pixels[(x-1) + (y+1)*image.width]) += (error*3) >> 4;
access(image.pixels[(x+0) + (y+1)*image.width]) += (error*5) >> 4;
access(image.pixels[(x+1) + (y+1)*image.width]) += (error*1) >> 4;
}
And this is how you use it:
RGBTriple* currentPixel = &(image.pixels[x + y*image.width]);
compute_disperse(currentPixel, AccessR);
compute_disperse(currentPixel, AccessG);
compute_disperse(currentPixel, AccessB);
Also: I would switch to using references rather than pointers, but I tried to get as close to the original approach as possible.
source to share