4 ms·
Well, you don't really need to mimic OO to make that code cleaner and less error prone. Here's what he wrote in order to complain about it: if (mode == am
by wott 5y ago
Well, you don't really need to mimic OO to make that code cleaner and less error prone.
Here's what he wrote in order to complain about it:
if (mode == ambient) {
// handle pixels as ambient...
int steps = right.a.x - left.a.x;
float dv = (right.a.ambientLight - left.a.ambientLight)/steps;
float currentIntensity = left.a.ambientLight;
for (i=left.a.x; i<right.a.x; i++) {
WorkOnPixelAmbient(i, dv);
currentIntensity+=dv;
}
} else if (mode == gouraud) {
// handle pixels as gouraud...
int steps = right.g.x - left.a.x;
float dred = (right.g.red - left.g.red)/steps;
float dgreen = (right.g.green - left.g.green)/steps;
float dblue = (right.g.blue - left.g.blue)/steps;
float currentRed = left.g.red;
float currentGreen = left.g.green;
float currentBlue = left.g.blue;
for (j=left.g.x; i<right.g.x; j++) {
WorkOnPixelGouraud(j, currentRed, currentBlue, currentGreen);
currentRed+=dred;
currentGreen+=dgreen;
currentBlue+=dblue;
}
Nobody should do that. We use a temporary variable of the correct type. For example this way (but it could also be done with pointers to access directly the union variable):
switch(mode) {
case AMBIENT: {
PixelDataAmbient l = left.a;
PixelDataAmbient r = right.a;
int steps = r.x - l.x;
float dv = (r.ambientLight - l.ambientLight)/steps;
/* ... */
break;
}
case GOURAUD: {
PixelDataGouraud l = left.g;
PixelDataGouraud r = right.g;
int steps = r.x - l.x;
/* ... */
break;
}
/* ... */
default:
show_error("Illegal mode");
}
There is no need to cart all the .a. and .g. around and clutter the code. And there is no possibility to accidentally access the wrong fields the wrong way: trying to access l.ambientLight in the second block (GOURAUD) will fail at compilation since a PixelDataGouraud doesn't have an ambientLight field. It is also not possible to do PixelDataGouraud l = left.a; since left.a is not a PixelDataGouraud.
Much cleaner, much more readable and avoids most of the possible errors he complains about.
- ActorNightly 5y agoYou can definitely do that, however in practice, separating out by files makes the code very easy to test, since you can validate functionality per pixel type, and then validate your outer loop by passing fake objects with correct functions. You can also have internal workings with static functions/variables that you can use as helpers, but invisible to outside code and not declared in header files.