>>1. Initializations of pointers being removed. >If you mean: > >act.comm.c, 'paper' is unconditionally > initialized later. act.informative.c: 'obj' > is also. > >I could keep going but you get the idea. If we > don't initalize it, think of it as a guarantee > that we won't just spontaneously use the > variable. It'll have something assigned to it > (later) that is used. This is a good case for C++ where you don't need to declare a variable until you actually need it. But we aren't doing C++ (yet), and I still don't like having uninitialized pointers lying around, even if we are "guarenteed" to not use it until we have initialized it. Just because *you* guarantee the stock code won't use until it is inited doesn't mean someone else won't come along and modify the code such that they try to use before your init, only it hasn't been initialized so they get a garbage pointer. With a null pointer the game will just crash and it is obvious there is a problem and where the problem is; with a garbage pointer the game may or may not crash and things may seem to be fine for a while and then you add more stuff and more stuff, and yet again still more stuff to the code and then it finally starts crashing in the old, bad code but it has been so long since you looked at that section and you can't even be sure any more that maybe you aren't getting a memory stomp in some of the newer code instead and you spend a whole day trying to track it down. Which has already happened to me :p It doesn't take any effort to set a pointer to NULL on declaration and it protects you from future problems when you change things in unexpected ways. Maybe you don't care to do it in the stock code and that is fine - you are the final arbiter of these matters, but I'm certainly doing it for my own code! >>2. Return statements at the end of void >>functions are being removed. > We're consistent, therefore they're useless. > Your code may become unconsistant, but then > you should fix the code, not add extra > 'return;' lines that may be at the wrong > indent level anyway. But you still aren't entirely consistent: void foo(...) { if (!GET_SKILL(ch, SKILL_whatever)) return; one_argument(arg, buf) if (!*buf) return; /* etc. etc. blah blah blah */ } You have returns for exiting the function in the middle of the function, but not at the end. That, to me, is inconsistent - you exit the function in some cases by using a return statement, but not in all cases. Obviuosly you can't not have the returns in the middle, so the only way to be consistent is to have it also at the end. In any event, the compiler *should* compile the statement out (i.e. no extra instructions will be generated), so it is really just a matter of preference. Again, you can do whatever you want with the stock code, but I prefer the self-consistency and so I'm certainly doing it for my own code (and all patches and snippets I upload :) I also just noted in the patch the following: for (zvn = atoi(value), zrn = 0; zone_table[zrn].number != zvn && zrn <= top_of_zone_table; zrn++); <-- semicolon at end of for loop :p if (zrn <= top_of_zone_table) print_zone_to_buf(buf, zrn); which looks at first glance like some mis- indented, i.e. broken, code. If you have a blank for loop, you really should comment it, so nobody reports it later as a bug :p for (zvn = atoi(value), zrn = 0; zone_table[zrn].number != zvn && zrn <= top_of_zone_table; zrn++) ; /* Just counting... */ Same thing for cases in a switch statement that you are having fall through to the next case, though I haven't explicitly seen that - it's just a very similar situation that I feel should always be commented when it is done. But, I admit I am just nitpicking on all presented issues here, so please don't hate me =) Angelfire for your free web-based e-mail. http://www.angelfire.com +------------------------------------------------------------+ | Ensure that you have read the CircleMUD Mailing List FAQ: | | http://qsilver.queensu.ca/~fletchra/Circle/list-faq.html | +------------------------------------------------------------+
This archive was generated by hypermail 2b30 : 04/10/01 PDT