Just because you can doesn't mean you should.
-
Hmmmm, Looks like Sebastiano Vigna[^] wrote that code back in 1998 shortly after leaving Milano. Best Wishes, -David Delaune
-
Any employee who thinks he has "job security" should be summarily fired.
-
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!
-
Any employee who thinks he has "job security" should be summarily fired.
Unless of course the job security is because of competence.
Wrong is evil and must be defeated. - Jeff Ello
-
Well, I searched for the code snippit and found it in the NE editor source code[^] dated 1998. Best Wishes, -David Delaune
-
Tripping through some older but still used C code, I found this section: action a; if ((a = hash_table[r]) && !cmdcmp(commands[--a].name, p) || (a = short_hash_table[r]) && !cmdcmp(commands[--a].short_name, p)) r = a; else r = -1; Somebody sure put a lot of faith that the order of evaluation, especially short-circuit evaluation, would remain the same across compilers! Of course, the programmer saved a couple of characters by excluding four(?) unnecessary parens. Upon further investigation, I found many instances of this type of statement structure. Apparently that was the preferred coding style. So, I'm guessing the programmer probably saved 100 characters. But it takes a lot of time to examine each statement and hopefully understand what is going on.
-
I hate that kind of stuff. I always add the redundant parenthesis because I want to be explicit about what is going on and I find it helps in deciphering the statement. I do NOT want to rely on the precedence order.
"They have a consciousness, they have a life, they have a soul! Damn you! Let the rabbits wear glasses! Save our brothers! Can I get an amen?"
Preach it brother!
Software Zen:
delete this; -
Unless of course the job security is because of competence.
Wrong is evil and must be defeated. - Jeff Ello
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; -
Tripping through some older but still used C code, I found this section: action a; if ((a = hash_table[r]) && !cmdcmp(commands[--a].name, p) || (a = short_hash_table[r]) && !cmdcmp(commands[--a].short_name, p)) r = a; else r = -1; Somebody sure put a lot of faith that the order of evaluation, especially short-circuit evaluation, would remain the same across compilers! Of course, the programmer saved a couple of characters by excluding four(?) unnecessary parens. Upon further investigation, I found many instances of this type of statement structure. Apparently that was the preferred coding style. So, I'm guessing the programmer probably saved 100 characters. But it takes a lot of time to examine each statement and hopefully understand what is going on.
-
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. :mad: 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.
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
-
Yup, I'm pro-parenthesis too. Real important in Regular Expressions as well.
Then you should use LISP! ROTFLMAO... I agree about NOT counting on short circuiting, and using parenthesis. In our coding standards we consider them the "White Space" if evaluations.
-
Tripping through some older but still used C code, I found this section: action a; if ((a = hash_table[r]) && !cmdcmp(commands[--a].name, p) || (a = short_hash_table[r]) && !cmdcmp(commands[--a].short_name, p)) r = a; else r = -1; Somebody sure put a lot of faith that the order of evaluation, especially short-circuit evaluation, would remain the same across compilers! Of course, the programmer saved a couple of characters by excluding four(?) unnecessary parens. Upon further investigation, I found many instances of this type of statement structure. Apparently that was the preferred coding style. So, I'm guessing the programmer probably saved 100 characters. But it takes a lot of time to examine each statement and hopefully understand what is going on.
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 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. :mad: 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.
Maybe he had read "How to write unmaintainable code" https://www.se.rit.edu/~tabeec/RIT_441/Resources_files/How%20To%20Write%20Unmaintainable%20Code.pdf[^]
-
Tripping through some older but still used C code, I found this section: action a; if ((a = hash_table[r]) && !cmdcmp(commands[--a].name, p) || (a = short_hash_table[r]) && !cmdcmp(commands[--a].short_name, p)) r = a; else r = -1; Somebody sure put a lot of faith that the order of evaluation, especially short-circuit evaluation, would remain the same across compilers! Of course, the programmer saved a couple of characters by excluding four(?) unnecessary parens. Upon further investigation, I found many instances of this type of statement structure. Apparently that was the preferred coding style. So, I'm guessing the programmer probably saved 100 characters. But it takes a lot of time to examine each statement and hopefully understand what is going on.
rjmoses wrote:
Somebody sure put a lot of faith that the order of evaluation, especially short-circuit evaluation, would remain the same across compilers!
Order of evaluation and the short-circuit behavior is (or at least, used to be) part of the C language definition. It's the use of variable assignment and pre-decrement operator in the (short-circuited) if statement that worries me. Regardless, I hope you rewrote it into something more maintainable.
I live in Oregon, and I'm an engineer.
-
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 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 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 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.
