On Wed, 20 Mar 2002 [EMAIL PROTECTED] wrote:
> Ok this is half a little snippet and half a suggestions e-mail. I have
> two handy functions called send_to_char_centered() and divide.
> Send_to_char_centered() will center a string of text to a screen 80
> spaces wide, while divide will use stcc to make a divider. The syntax for
> send_to_char_centered is: send_to_char_centered( "Hello world!", ch );
> And for divide it's: divide( 40, ch ); This will make:
> ----------------------------------------
> The code for the two are right here. So far I havn't found any bugs
> except if you try to go over 78 '---'s in divide it will crash the MUD. I
> just know SOMETHING has to be more wrong than that. Any suggestions for
> improvement guys? Here's the code (Yes it's in semi-C++ for now):
>
There are several things I would like to point out real quick...
> /**********************************************************************
> File: Util.cpp Description: Various text manipulation utilities
> Author: Devin Torres Date: March 20, 2002
> **********************************************************************/
>
> /**********************************************************************
> Needed includes.
> **********************************************************************/
> #include <cstdio>
> #include <ctime>
> #include <string>
> #include "merc.h"
#include <stdio.h>
#include <time.h>
#include <string.h>
#include "merc.h"
I can see why it was so difficult to get all the C++ out before posting
this code. It took me almost 10 seconds to do it! :)
> /**********************************************************************
> Center a string of text.
> **********************************************************************/
> void send_to_char_centered( char *txt, CHAR_DATA *ch )
> {
> char buf[MAX_STRING_LENGTH];
> short int length;
>
> buf[0] = '\0';
> length = strlen( txt );
>
> if ( length > 78 )
> length = 78;
>
> if ( length < 1 )
> length = 1;
>
> length /= 2;
> length = 40 - length;
>
> do
> {
> strcat( buf, " " );
> length--;
> } while ( length != 0 );
Poor algorithm design. This runs in O(length squared) time. strcat has to
scan the string every time to find out where the end of it is, so it's
quite inefficient to use it in a loop like this.
Since length isn't going to be that big, it really isn't that big of a deal,
but it's also not difficult to have a much more efficient O(length)
algorithm:
int i;
for ( i = 0; i < length; i++ )
{
buf[i] = ' ';
}
strcpy ( buf + i, txt );
>
> strcat( buf, txt );
> send_to_char( buf, ch );
> }
There's a possible bug here: You need to make sure strlen(txt) is never
longer than MAX_STRING_LENGTH - 2. Either in code or at least a comment
to the user of the function.
> /**********************************************************************
> Build a centered divider for text.
> **********************************************************************/
> void divide( short int length, CHAR_DATA *ch )
> {
> char buf[MAX_STRING_LENGTH];
>
> buf[0] = '\0';
>
> if ( length > 78 )
> length = 78;
>
> if ( length < 1 )
> length = 1;
>
> do
> {
> strcat( buf, "-" );
> length--;
> } while ( length != 0 );
>
> send_to_char_centered( buf, ch );
> }
The same comments apply here: Using strcat in a loop like this is a poor
algorithm. The same example algorithm given above could also be used here.
And since you're using this same algorithm (copy a character X times)
multiple times, why not make it into a function? You might find some
other places it could be useful. I think I've seen a function that
does something like that before. But I can't remember where I saw it,
or if it was even in C.
I also have another suggestion: Don't make too many assumptions about
your players and their clients. Who says they're using 80 columns?
They might not be, and there's no reason to force them to.
Perhaps add a variable that they can set to their terminal width,
similar to the variable ROM uses to keep track of terminal heights for
paging output.
Dennis
