|
I once had a colleague who believed in obfuscating his C code to the maximum. He did not add comments to his code or any documentation of any kind. I believe he thought if he was the only one to understand his code, it provided a kind of job security.
All went well for him until I was promoted into a position where he reported to me. One of my first actions was to fire him.
|
|
|
|
|
|
In the name of the code-base, the style guidelines, and the future maintainers, we say - comment (And just name things well.)
|
|
|
|
|
Any employee who thinks he has "job security" should be summarily fired.
|
|
|
|
|
Graveyards are filled with people who were indispensable.
|
|
|
|
|
Unless of course the job security is because of competence.
|
|
|
|
|
Competence is not a significant factor in job security according to my experience. We've had layoffs every 6-9 months for the last several years. In that time my team has gone from 17 down to 5, although now it's back up to 6. The most common factor in the layoffs was which product you were on, followed by productivity, followed by age/salary.
Note that competence and productivity are not equivalent.
Software Zen: delete this;
|
|
|
|
|
I'm retired now, but when I was the senior developer coding for job security was grounds for termination. Over the course of my career, I spent far, far too much time decipheriing and rewriting such code to be understandable.
It's a hard life, but somebody's got to live it if only to act as an inspiration to others.
- Dan Best
|
|
|
|
|
|
Hmmmm,
Looks like Sebastiano Vigna[^] wrote that code back in 1998 shortly after leaving Milano.
Best Wishes,
-David Delaune
|
|
|
|
|
|
|
Good one! I deserved that.
How did you come up with S. Vigna?
|
|
|
|
|
Well,
I searched for the code snippit and found it in the NE editor source code[^] dated 1998.
Best Wishes,
-David Delaune
|
|
|
|
|
Very good! I like ne and it is an excellent editor! My hat is off to Sebastian. I was using it back in the late 1990's and adapted to run on AIX when I was doing work out of Chicago in Australia and London over a 9600BPS links. I had cause to want to run it recently for a project and resurrected it.
I love it's ease of use and functionality, but the code is difficult to follow.
I would love to meet Sebastian--he must be one brilliant son of a gun!
|
|
|
|
|
Just out of curiosity, how did you search? And where?
|
|
|
|
|
|
I agree with you that the code is just a pain.
Quote: Somebody sure put a lot of faith that the order of evaluation, especially short-circuit evaluation, would remain the same across compilers!
I would. It better. If it's not, it's not C spec and the documentation better have that in big red flashing letters.
When I was growin' up, I was the smartest kid I knew. Maybe that was just because I didn't know that many kids. All I know is now I feel the opposite.
|
|
|
|
|
Oh, ever hear of Caché? (The "database".)
Its not-quite-SQL language doesn't honor order-of-operations! (But it's faaaasssst!)
|
|
|
|
|
that seems kind of pointless. How can I tell what it's doing? oh never mind. it's just silly. i don't even want to know.
When I was growin' up, I was the smartest kid I knew. Maybe that was just because I didn't know that many kids. All I know is now I feel the opposite.
|
|
|
|
|
Quote: Somebody sure put a lot of faith that the order of evaluation, especially short-circuit evaluation, would remain the same across compilers! That's given.
However I agree with you, it is difficult to understand such a code.
|
|
|
|
|
"Somebody sure put a lot of faith that the order of evaluation, especially short-circuit evaluation, would remain the same across compilers!"
Evaluation order must stay the same as it is part of the C language spec since its inception!
|
|
|
|
|
Both the operator precedence (and thus the order of evaluation) and short circuiting are part of the C language definition and is not compiler dependent. The compiler can only reorder the expression during optimization if it can ensure that it does not change the result.
It is quite safe to rely upon. This expression already has a sufficient number of parentheses, more just makes it less readable. I'm not even sure where you would even put extra parentheses.
|
|
|
|
|
I read the spec and it supposed to be standard across all compilers but I have lost count of how many OS and application bugs of that type I have had to chase.
As patbob pointed out, it is the pre-decrement that is worrisome. And the first couple of times I look at the code, I missed the "!".
To test this logic, I wrote a little program which (I think) simulates the original code results:
#include <stdio.h>
int testf(int rv, int a) {
return rv;
}
int main()
{
int value = 0;
for (int i = 0; i < 16; i++) {
printf("%2x = ", value);
int a = 0;
if (value & 8) printf("T"); else printf("F");
if (value & 4) printf("T"); else printf("F");
if (value & 2) printf("T"); else printf("F");
if (value & 1) printf("T"); else printf("F");
printf(" = ");
if ( (value & 8) && testf((value & 4), --a) || (value & 2) && testf((value & 1), --a))
printf("T %d", a);
else
printf("F %d", a);
value++;
printf("\n");
}
return(0);
}
The results are:
0 = FFFF = F 0
1 = FFFT = F 0
2 = FFTF = F -1
3 = FFTT = T -1
4 = FTFF = F 0
5 = FTFT = F 0
6 = FTTF = F -1
7 = FTTT = T -1
8 = TFFF = F -1
9 = TFFT = F -1
a = TFTF = F -2
b = TFTT = T -2
c = TTFF = T -1
d = TTFT = T -1
e = TTTF = T -1
f = TTTT = T -1
|
|
|
|
|
I stand corrected. I missed the double decrement (sorry, it has been a long and intense semester, I'm not fully back up to speed). Multiple increments or decrements of the same variable are undefined according to the standard. Sequencing might make it OK, but that can't be assured. There might be a corner of the standard that allows it with the sequencing, but I wouldn't assume that it is legal.
The intent of the original logic appears to be: Given a key r, to search two hash tables, in order. If a match is found, then r should be assigned the index returned from the matching hash table, reduced by one for indexing into the "commands" table.
The difference between your code and the original is that the original reassigns "a" after the "or". That is intended to wipe out any previous changes to "a". Your code does not do that so it will potentially get a double increment.
To match the intent of the original, one of the following tweaks could be used (minor formatting applied -- note that I use "and", "or" and "not" to reduce errors caused by mixing up & and &&, or by mixing up | and ||).
action a;
if ((a = hash_table[r]) and not cmdcmp(commands[--a].name, p)
r = a;
else if (a = short_hash_table[r]) and not cmdcmp(commands[--a].short_name, p))
r = a;
else
r = -1; This has an extra assignment -- that the compiler can optimize away by merging identical blocks. A more interesting alternative is
action a;
if ((a = hash_table[r]) and (a = a - 1, not cmdcmp(commands[a].name, p)) or
(a = short_hash_table[r]) and (a = a - 1, not cmdcmp(commands[a].short_name, p)))
r = a;
else
r = -1; which makes use of the comma operator. Decrement is not directly used, but the compiler will generate the same code, but lifted before evaluating the function call's parameters.
|
|
|
|