Idiotic programming style edicts
in [General Programming]
|
From: Nick Keighley on 24 Apr 2010 08:32 On 16 Mar, 19:14, ImpalerCore <jadil...(a)gmail.com> wrote: > On Mar 16, 12:51 pm, Jonathan de Boyne Pollard <J.deBoynePollard- > newsgro...(a)NTLWorld.COM> wrote: > > >> If an obscure technique is being employed, then a comment to that > > >> effect is a helpful pointer but I have seen _REALLY USEFUL_ comments > > >> along the following lines ... > > > >> x := X + 1 ; increment x > > > >> ... which only serve to increase the noise floor. > > > > In our shop, those appeared when we got the idiotic edict that each > > > line had to have a comment. > > > Such edicts make one want to write code in the form > > > x /* The variable x */ > > = /* is assigned */ > > x /* its value * / > > + /* plus * / > > 2 /* one */ > > ; /* . */ > > It also shows that maintaining these kinds of comment are more trouble > than the comment is worth. The maintenance of these comments is way > higher than the information they give, i.e. Who forgot to change the '/ > * one */' to '/* two */'? this is because the comment is a violation of what I used to call Normal Form for code. "Don't keep the same data in more than one place". But has a more modern mnemonic: DRY "Done Repeat Yourself". If the code is clear the comment is unnecessary.
From: spinoza1111 on 24 Apr 2010 12:03 On Apr 24, 8:26 pm, Nick Keighley <nick_keighley_nos...(a)hotmail.com> wrote: > On 24 Apr, 07:39,spinoza1111<spinoza1...(a)yahoo.com> wrote: > > > On Mar 21, 2:17 am, Seebs <usenet-nos...(a)seebs.net> wrote: > > <snip> > > > > > > > > [...] I also program in a couple of languages, including C. > > > >http://github.com/wrpseudo/pseudo > > > This is the Coding Horror with [...] > > the unnecessary and unstructured use of switch which > > made it appear that you needed to handle ack and nak but did not. I'm > > afraid it is unprofessional work. > > > switch (msg->type) { > > case PSEUDO_MSG_PING: > > msg->result = RESULT_SUCCEED; > > if (opt_l) > > pdb_log_msg(SEVERITY_INFO, msg, program, tag, NULL); > > return 0; > > break; > > case PSEUDO_MSG_OP: > > return pseudo_op(msg, program, tag); > > break; > > case PSEUDO_MSG_ACK: /* FALLTHROUGH */ > > case PSEUDO_MSG_NAK: /* FALLTHROUGH */ > > default: > > pdb_log_msg(SEVERITY_WARN, msg, program, tag, "invalid message"); > > return 1; > > } > > I'm pretty sure the original code was mine. It's made up code to > illustrate that C doesn't have any other way to use the same code to > process multiple items. Except duplicating the code which I regard as > an even greater sin. This idiom makes it plain that ACKs and NAKs > haven't simply been forgotten. Duplicating FUNCTION CALLS (inline or not) isn't a "sin". It's abstraction 101 and reluctance to do so (using predefined symbols if the parameters have as it happens the same value) is a sign of programming childishness. The above code does not demonstrate that the programmer "thought of" ACK and NAK. It looks like he forgot how and/ or whether to handle them. Peter (and you), if you think you are professional programmer should AVOID idioms such as fallthrough which are unique today to C, and use switch() in a structured form. This is because you're responsible to program maintainers who because they are smarter than you and have a work ethic, have troubled to learn languages like C Sharp and Java. The code should be an if statement for it is simply not reflective of the logical situation. It is a vanity use of switch() which displays that you or Peter want everyone to think you're cool. We don't think you're cool. > > It's a pretty common idiom where I work. > > > It has been cleaned up since I looked at it last, perhaps owing to my > > comments earlier this year. Now it's more obvious that you mean to say > > that ACK and NAK are invalid. However, it's still unstructured > > fallthrough and it makes a professional and knowledgeable programmer > > wonder if you're missing some code for ACK and NAK. > > hence the comment... Though the idiom is so widespread I don't think > it's really necessary. > > > > > > > > > switch (msg->op) { > > case OP_FCHOWN: /* FALLTHROUGH */ > > case OP_FCHMOD: /* FALLTHROUGH */ > > case OP_FSTAT: > > prefer_ino = 1; > > pseudo_debug(2, "%s %llu [%s]: ", pseudo_op_name(msg->op), > > (unsigned long long) msg->ino, > > msg->pathlen ? msg->path : "no path"); > > break; > > default: > > pseudo_debug(2, "%s %s [%llu]: ", pseudo_op_name(msg->op), > > msg->pathlen ? msg->path : "no path", > > (unsigned long long) msg->ino); > > break; > > } > > > Here's the same somewhat cleaned up practice, but here you should have > > defined the assignment of prefer_ino, the pseudo_debug and the break > > as a macro. Better yet you should have used an OR statement. > > could you illustrate the use of an OR statement with an example? Sure: and this rewrite indicates something even probably more horrible and less professional about this code It is that despite the fact that you or Peter claim to want to use switch with fall thru "to avoid repeated code" (which ignores a basic form of abstraction), your switch statement has repeated code! The two calls to pseudo_debug2 do the same thing, and one of them appears to get the parameters out of order! After two months work, this is unacceptable and makes a sick joke out of Peter's libel of Schildt. if (msg->op == OP_FCHOWN || msg_op == OP_FCHMOD || msg_op == FSTAT). prefer_ino = 1; pseudo_debug(2, "%s %llu [%s]: ", pseudo_op_name(msg->op), (unsigned long long) msg->ino, msg->pathlen ? msg->path : "no path"); // If this, indeed, is the right order Even if it is possible to call pseudo_debug in one case with an unsigned 64 bit second parameter, and a string in the other, if for some reason the code in pseudo_debug supports this polymorphism, it is simply unacceptable.
From: Jennifer Usher on 24 Apr 2010 13:35 "Kenneth Brody" <kenbrody(a)spamcop.net> wrote in message news:7rWdnTnBRIhYWEzWnZ2dnUVZ_i2dnZ2d(a)bestweb.net... >> Yes, but most supervisors who expect lots of comments also have no sense >> of humor. I once wrote some code for use by a couple of programmers who >> had claimed what I did could not be done. I wrote the comments in the >> form of a fairy tale. They were not amused, as it was pretty >> condescending, but it did make it very clear how to implement what I had >> created. And the comments were almost as fun to write as the code. > > Just be careful to not be _too_ condescending. Look what happened to > Galileo after publishing "Dialogo Sopra I Due Massimi Sistemi del Mondo". This is true. -- Jennifer Usher
From: Seebs on 24 Apr 2010 14:39 On 2010-04-24, Nick Keighley <nick_keighley_nospam(a)hotmail.com> wrote: > I'm pretty sure the original code was mine. Don't think so -- he jumped on this in pseudo, where it's been there for months. > It's made up code to > illustrate that C doesn't have any other way to use the same code to > process multiple items. Except duplicating the code which I regard as > an even greater sin. This idiom makes it plain that ACKs and NAKs > haven't simply been forgotten. Exactly. The real key point is that, in his initial "analysis", Nilges got his explanation of what the code does *completely wrong*. He claimed that those two cases had no code and didn't do anything, and that this should have been commented. You see what happens next; he didn't know about fallthrough in C's switch statement. That could make him look bad. The only way out is to declare that the thing which could make him look bad is so awful that it should never have been used. > hence the comment... Though the idiom is so widespread I don't think > it's really necessary. It's not if we assume a reader who knows C. However, in something I'm distributing as open source, I prefer not to make assumptions. There are limits, though -- I don't mind commenting switches with "fallthrough" because occasionally people just plain forget that, or get distracted by the layout. In general, though, I am gonna write code that's reasonably clear, and following the advice Nilges gives does not produce reasonably clear code, as witness the fact that his style leaves him spending days working on simple problems. -s -- Copyright 2010, all wrongs reversed. Peter Seebach / usenet-nospam(a)seebs.net http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
From: Kenny McCormack on 24 Apr 2010 15:27
In article <09b14a6e-23bb-4fc0-8d66-1e7164746293(a)h16g2000prf.googlegroups.com>, spinoza1111 <spinoza1111(a)yahoo.com> wrote: .... >Peter (and you), if you think you are professional programmer should >AVOID idioms such as fallthrough which are unique today to C, and use Quibble (thank you, mr thompson): fallthrough is not unique to C. WinBatch has it also. In a high level language like WB, it is actually quite nice and useful. But in a low level langauge (C), it is too dangerous for the day-to-day programmer. -- > No, I haven't, that's why I'm asking questions. If you won't help me, > why don't you just go find your lost manhood elsewhere. CLC in a nutshell. |