Bye bye, Persona Holder

A nasty little structure that cluttered up lots of the cell code; basically the persona holder was a little wrapper that held a pointer to a Persona and pointers to the “prev” and “next” holders – although the ordering was entirely arbitrary. It also used up memory to provide such extremely useful fields as “isPersonaHolder” (which presumably hoped that other data structures would honor the code and have a “false” value at this poisition in them) and “isPersonaNull” – which was supposed to be TRUE if the pointer was NULL and FALSE otherwise, but ran into some issues where the pointer got changed without concern for it. It also duplicated information from the personaID, but you were only supposed to use those fields… if ( isPersonaNull == FALSE ). For example

UINT32 hostGetPersonaID(PERSONA_HOLDER* holder)  
{  
  if ( holder == NULL  
      || holder->isPersonaNull == TRUE  
      || holder->persona == NULL )  
    return 0 ;  
  else  
    return holder->personaID ;  
}

You wouldn’t believe the amount of code dedicated to managing, debugging, and massaging this structure… All the headaches of trying to make sure that we have a valid holder before we can get at the Persona pointer we really wanted…

Clearly someone had too much time on their hands to have put this much effort into mommying a pointer.

Bring on the typedef PERSONAS std::vector; and out with a good 14-16 functions just for very standard list operations with PERSONA_HOLDERS. Out with about ~300 or so lines of code dealing with going from a PERSONA_HOLDER to a Persona*, mostly in one file, and out with a lot of code that just plain cluttered up what the code was trying to do.

And I replaced about 130 lines of code that allocated and operated the “free” list of personas with a simple std::stack. Added a scattering of test harness code and spent a few hours testing. Score one for legible code!

More importantly, it’s one of the many steps I need to take in getting the new grid system I wrote as the basis for STOs into the cell host to replace the current grid/update system. Hooah.

2 Comments

There’s something to be said for competency. Good to know you are cleaning “code” house again!

Heh, “isPersonaNull” seems like a minor WTF (http://www.thedailywtf.com/) or someone being paid by LOC. Either the pointer is NULL or it isn’t and there’s not a darn thing that bool can do to change it ;)

Once you go STL you never go back. I still have nightmares about realloc…

Leave a Reply

Name and email address are required. Your email address will not be published.

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

You may use these HTML tags and attributes:

<a href="" title="" rel=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <pre> <q cite=""> <s> <strike> <strong> 

%d bloggers like this: