5 ms·

This programmer, Mr. Ruud Helderman, is like Cool McCool, "Danger is my business!". For example, the parser, function matchParam: OBJECT *obj; par->tag = s

This programmer, Mr. Ruud Helderman, is like Cool McCool, "Danger is my business!". For example, the parser, function matchParam: OBJECT *obj; par->tag = src; par->distance = *src == '\0' ? distNoObjectSpecified : distUnknownObject; forEachObject(obj) ... The initialization of the obj variable received an obliviate spell. Also function parseAndExecute (). The way he deal with invalid input is, epic: static const COMMAND commands[] = { {executeQuit , "quit"}, {executeLookAround, "look"}, ... } for (cmd = commands; !matchCommand(input, cmd->pattern); cmd++); return (*cmd->function)();

jstimpfle 7y ago
This is C89 style code where you couldn't declare the loop variables in the for loop, as in "for (OBJECT * obj = ... ". There's no problem with this code. The obj variable is initialized in the forEachObject macro which expands to a for loop. (Pretty damn sure - I haven't actually looked up the definition). The array iteration code is also solid. Seems like a pretty good programmer, judging from the code you cite here at least. While I personally like to put a few assertions just to find my typos quicker, adding additional fluff here is mostly detrimental. The kind of bugs that can happen with this sort of "dangerous" code are the ones that you basically just typos that you catch on the first run. I.e. it's not like there are any rarely occurring edge cases that are unhandled, since the code is extremely straightforward. The "complexity" is very low.
- gustavorg 7y ago
  Overall I agree with you, my comment is just pointing out that this kind of tricks are dangerous, Cool McCool style, use at your own risk and so. Things you stop doing after your first million bugs fixed.
  - jstimpfle 7y ago
    Well in fact I've tended more and more to this bare kind of coding as I've aged (and hopefully gained experience). Many of the seatbelts (like assertions) I've actually hit at some point or another (so they turned out to be useful in these situations), but on the other hand they allowed me to be more sloppy in my thinking, writing more complicated code, which I feel could be a net loss in productivity. Then again this is just some example code on a website, so it's natural that it has to be done a little more straightforwardly than you would write it in actual practice.
    - zelphirkalt 7y ago
      It's natural to show people code as an example, which not write like that in practice? What's the example for then? Is there a disclaimer following, saying thaT the code is not how you would write it and mentioning all the things you would do differently? Enlighten me please.
- kbumsik 7y ago
  > The array iteration code is also solid. Shouldn't there be a NULL at the end of the array and do a null check in the iteration?
  - pm215 7y ago
    I think the last array entry {executeNoMatch , "A?"} will match on any string (and print the fallback "I don't know how to VERB" message), so it's impossible to fall off the end of the array. If you wanted to use a more defensive-programming style then you could add a sentinel of some kind and assert that you never hit it.
mulle_nat 7y ago
How do you feel qualified to critique someone else's C code, even publicly on Hacker News ? You seem to have basic problems with understanding how a macro (in this case forEachObject) works.
emily-c 7y ago
You see similar code to his unconditional executeNoMatch at the end of the table in table based code all the time. Additionally forEachObject is a macro that immediately initializes a variable. For projects like this the code is completely fine. I'm not sure what is particularly dangerous about this code for a small to medium size project.
0xff00ffee 7y ago
You have people literally defending uncommented, unclear code in your replies. There are phases programmers go through. One phase occurs about after 10 years of professional programming. I call this the "no mercy" phase, where these newly-minted senior programmers think defensive programming is for noobs, and scrunch their C code down to the fewest number of lines and characters. And when someone objects, the reply is, paraphrased (as seen in the replies): "You're just not as good a programmer as me." Sadly, it takes another 10 years for this arrogance to dissipate.
- jstimpfle 7y ago
  If you mean me, you're right in that I've been working with C in varying capacity for just about 10 years (Not that I believe that "years" is a good unit to measure experience). On the other hand, I miss from your comment what exactly is wrong with my argumentation, and I feel that the arrogant comment is not mine but yours (assuming you have 20 years or more).
- emily-c 7y ago
  Most legacy code is this way unfortunately. The actual techniques employed by the "called out" code are not uncommon. I don't think that people are defending the lack of clarity but rather what the code is doing itself. It could be written more clearly, though one could argue that the lengthy series of articles does just that. Maybe it's an exercise for you to write it how you want. Have you taken a look at some major C codebases? It's not justification but an appeal to authority seems pretty unnecessary :)

jstimpfle 7y ago

This is C89 style code where you couldn't declare the loop variables in the for loop, as in "for (OBJECT * obj = ... ". There's no problem with this code. The obj variable is initialized in the forEachObject macro which expands to a for loop. (Pretty damn sure - I haven't actually looked up the definition). The array iteration code is also solid. Seems like a pretty good programmer, judging from the code you cite here at least. While I personally like to put a few assertions just to find my typos quicker, adding additional fluff here is mostly detrimental. The kind of bugs that can happen with this sort of "dangerous" code are the ones that you basically just typos that you catch on the first run. I.e. it's not like there are any rarely occurring edge cases that are unhandled, since the code is extremely straightforward. The "complexity" is very low.

gustavorg 7y ago
Overall I agree with you, my comment is just pointing out that this kind of tricks are dangerous, Cool McCool style, use at your own risk and so. Things you stop doing after your first million bugs fixed.
- jstimpfle 7y ago
  Well in fact I've tended more and more to this bare kind of coding as I've aged (and hopefully gained experience). Many of the seatbelts (like assertions) I've actually hit at some point or another (so they turned out to be useful in these situations), but on the other hand they allowed me to be more sloppy in my thinking, writing more complicated code, which I feel could be a net loss in productivity. Then again this is just some example code on a website, so it's natural that it has to be done a little more straightforwardly than you would write it in actual practice.
  - zelphirkalt 7y ago
    It's natural to show people code as an example, which not write like that in practice? What's the example for then? Is there a disclaimer following, saying thaT the code is not how you would write it and mentioning all the things you would do differently? Enlighten me please.
kbumsik 7y ago
> The array iteration code is also solid. Shouldn't there be a NULL at the end of the array and do a null check in the iteration?
- pm215 7y ago
  I think the last array entry {executeNoMatch , "A?"} will match on any string (and print the fallback "I don't know how to VERB" message), so it's impossible to fall off the end of the array. If you wanted to use a more defensive-programming style then you could add a sentinel of some kind and assert that you never hit it.

mulle_nat 7y ago

How do you feel qualified to critique someone else's C code, even publicly on Hacker News ? You seem to have basic problems with understanding how a macro (in this case forEachObject) works.

emily-c 7y ago

You see similar code to his unconditional executeNoMatch at the end of the table in table based code all the time. Additionally forEachObject is a macro that immediately initializes a variable. For projects like this the code is completely fine. I'm not sure what is particularly dangerous about this code for a small to medium size project.

0xff00ffee 7y ago

You have people literally defending uncommented, unclear code in your replies. There are phases programmers go through. One phase occurs about after 10 years of professional programming. I call this the "no mercy" phase, where these newly-minted senior programmers think defensive programming is for noobs, and scrunch their C code down to the fewest number of lines and characters. And when someone objects, the reply is, paraphrased (as seen in the replies): "You're just not as good a programmer as me." Sadly, it takes another 10 years for this arrogance to dissipate.

jstimpfle 7y ago
If you mean me, you're right in that I've been working with C in varying capacity for just about 10 years (Not that I believe that "years" is a good unit to measure experience). On the other hand, I miss from your comment what exactly is wrong with my argumentation, and I feel that the arrogant comment is not mine but yours (assuming you have 20 years or more).
emily-c 7y ago
Most legacy code is this way unfortunately. The actual techniques employed by the "called out" code are not uncommon. I don't think that people are defending the lack of clarity but rather what the code is doing itself. It could be written more clearly, though one could argue that the lengthy series of articles does just that. Maybe it's an exercise for you to write it how you want. Have you taken a look at some major C codebases? It's not justification but an appeal to authority seems pretty unnecessary :)