Representative Comment

Cyrus

I work for a small company maintaining legacy code on an embedded device that was written by one of the co-founders of the company. How he got the code or electronics design to even boot is a mystery to this day, but I get to find all sorts of treasures left behind even three years on.

Being an embedded device you always hope for comments explaining the complex manipulation of peripherals, but sometimes, you just accept what you can get:

// KeyPress expects timer to be initialized.. and it was done above.
// Returns TRUE if buttons pressed .. FALSE otherwise.
// COMMENT: all this is very obscure coding to save flash space.

I'm still trying to figure out why he did what he did following this, but hey, at least I had fair warning.

Mole

Must have been using a crap compiler, or didn't bother to check the compiler output. As I find the difference between very readable code and obscure to-save-flash-space code marginal at best. That and the fact that if we think we'll get anywhere near the maximum capacity of the chip, we move to it way before then, as the price difference is negligable.

You certainly shouldn't have oscure coding for a function called such things as "IsKeyPressed". Thats just lame coding. The coder would be hauled over burning coal for that.

pbean

@Cyrus said:

I'm still trying to figure out why he did what he did ...

@Cyrus said:

// COMMENT: all this is very obscure coding to save flash space.

Arancaytar

If you're running into space problems while polling a key, something is wrong.

Cyrus

@pbean said:

@Cyrus said:
I'm still trying to figure out why he did what he did ...
@Cyrus said:
// COMMENT: all this is very obscure coding to save flash space.

Normally, I'd give you the point, but allow me to show one of his favorite coding patterns:

int foo = FALSE; // assume FALSE

if (bar >= 60)

foo = TRUE;

return foo;

He claims it saves processing time; you can imagine what he does to "save flash space".

Xyro

@Cyrus said:

int foo = FALSE; // assume FALSE
if (bar >= 60)
foo = TRUE;
return foo;

He claims it saves processing time; you can imagine what he does to "save flash space".

Hmm, well, if the scope of [code]foo[/code] is such that it can be statically initialized to FALSE at load time (like a value on the stack), then you'd only have to do a store operation sometimes instead of all the time if the statement was instead [code]foo = bar [/code]>[code]= 60[/code]... I think. At least, as long as the branch pipeline assumes the if will be true. That might save a nanosecond or two if the compiler doesn't help you out... Of course, the time it takes you to shake your head over this code will never be less than the accumulated amount of time that his code saved. Net loss!

A game for the lolz: see if you can get him to prove it empirically.

EDIT: wait, wouldn't that code actually use more flash space? Since it's the value of foo is already statically allocated? Time-space wars: "You (might) win (a little, maybe) this time, Time, but the battle is not over!" -- Space

Mason_Wheeler

@Xyro said:

@Cyrus said:
int foo = FALSE; // assume FALSE
if (bar >= 60)
foo = TRUE;
return foo;

He claims it saves processing time; you can imagine what he does to "save flash space".
Hmm, well, if the scope of foo is such that it can be statically initialized to FALSE at load time (like a value on the stack), then you'd only have to do a store operation sometimes instead of all the time if the statement was instead foo = bar >= 60... I think. At least, as long as the branch pipeline assumes the if will be true. That might save a nanosecond or two if the compiler doesn't help you out... Of course, the time it takes you to shake your head over this code will never be less than the accumulated amount of time that his code saved. Net loss!
A game for the lolz: see if you can get him to prove it empirically.
EDIT: wait, wouldn't that code actually use more flash space? Since it's the value of foo is already statically allocated? Time-space wars: "You (might) win (a little, maybe) this time, Time, but the battle is not over!" -- Space

But wouldn't writing it as a one-liner save both processing time and space, like this?

return (bar >= 60);

Xyro

@Mason Wheeler said:

But wouldn't writing it as a one-liner save both processing time and space, like this?
return (bar >= 60);

Oh yeah, it returns foo right there, so it shouldn't even have to store anything. Well that's just terrible code.