refactor(namekey): Reduce code duplication in NameKeyGenerator functions, misc formatting tweaks

Author: xezon

GeneralsMD/Code/GameEngine/Include/Common/NameKeyGenerator.h

@@ -36,22 +36,20 @@
 #include "Common/AsciiString.h"
 
 //-------------------------------------------------------------------------------------------------
-/**
-	Note that NameKeyType isn't a "real" enum, but an enum type used to enforce the
-	fact that NameKeys are really magic cookies, and aren't really interchangeable
-	with ints. NAMEKEY_INVALID is always a legal value, but all other values are dynamically
-	determined at runtime. (The generated code is basically identical, of course.)
-*/
+// Note that NameKeyType isn't a "real" enum, but an enum type used to enforce the
+// fact that NameKeys are really magic cookies, and aren't really interchangeable
+// with integers. NAMEKEY_INVALID is always a legal value, but all other values are dynamically
+// determined at runtime. (The generated code is basically identical, of course.)
 //-------------------------------------------------------------------------------------------------
 enum NameKeyType CPP_11(: Int)
 {
 	NAMEKEY_INVALID					= 0,
-	NAMEKEY_MAX							= 1<<23,					// max ordinal value of a NameKey (some code relies on these fitting into 24 bits safely)
-	FORCE_NAMEKEYTYPE_LONG	= 0x7fffffff	// a trick to ensure the NameKeyType is a 32-bit int
+	NAMEKEY_MAX							= 1<<23, // max ordinal value of a NameKey (some code relies on these fitting into 24 bits safely)
+	FORCE_NAMEKEYTYPE_LONG	= 0x7fffffff, // a trick to ensure the NameKeyType is a 32-bit int
 };
 
 //-------------------------------------------------------------------------------------------------
-/** A bucket entry for the name key generator */
+// A bucket entry for the name key generator
 //-------------------------------------------------------------------------------------------------
 class Bucket : public MemoryPoolObject
 {
@@ -72,12 +70,12 @@ inline Bucket::Bucket() : m_nextInSocket(NULL), m_key(NAMEKEY_INVALID) { }
 inline Bucket::~Bucket() { }
 
 //-------------------------------------------------------------------------------------------------
-/** This class implements the conversion of an arbitrary string into a unique
-	* integer "key". Calling the nameToKey() method with the same string is
-	* guaranteed to return the same key. Also, all keys generated by an
-	* instance of this class are guaranteed to be unique with respect to that
-	* instance's catalog of names.  Multiple instances of this class can be
-	* created to service multiple namespaces. */
+// This class implements the conversion of an arbitrary string into a unique
+// integer "key". Calling the nameToKey() method with the same string is
+// guaranteed to return the same key. Also, all keys generated by an
+// instance of this class are guaranteed to be unique with respect to that
+// instance's catalog of names. Multiple instances of this class can be
+// created to service multiple namespaces.
 //-------------------------------------------------------------------------------------------------
 class NameKeyGenerator : public SubsystemInterface
 {
@@ -99,16 +97,14 @@ class NameKeyGenerator : public SubsystemInterface
 	NameKeyType nameToKey(const char* name);
 	NameKeyType nameToLowercaseKey(const char *name);
 
-	/**
-		given a key, return the name. this is almost never needed,
-		except for a few rare cases like object serialization. also
-		note that it's not particularly fast; it does a dumb linear
-		search for the key.
-	*/
+	// given a key, return the name. this is almost never needed,
+	// except for a few rare cases like object serialization. also
+	// note that it's not particularly fast; it does a dumb linear
+	// search for the key.
 	AsciiString keyToName(NameKeyType key);
 
-  // Get a string out of the INI. Store it into a NameKeyType
-  static void parseStringAsNameKeyType( INI *ini, void *instance, void *store, const void* userData );
+	// Get a string out of the INI. Store it into a NameKeyType
+	static void parseStringAsNameKeyType( INI *ini, void *instance, void *store, const void* userData );
 
 private:
 
@@ -124,6 +120,7 @@ class NameKeyGenerator : public SubsystemInterface
 
 	NameKeyType nameToKeyImpl(const char* name);
 	NameKeyType nameToLowercaseKeyImpl(const char *name);
+	NameKeyType createNameKey(UnsignedInt hash, const AsciiString& name);
 
 	void freeSockets();
 

GeneralsMD/Code/GameEngine/Include/Common/Upgrade.h

@@ -235,10 +235,10 @@ class UpgradeCenter : public SubsystemInterface
 	void reset( void );												///< subsystem interface
 	void update( void ) { }										///< subsystem interface
 
-	UpgradeTemplate *firstUpgradeTemplate( void );	///< return the first upgrade template
-	const UpgradeTemplate *findUpgradeByKey( NameKeyType key ) const;		///< find upgrade by name key
-	const UpgradeTemplate *findUpgrade( const AsciiString& name ) const;				///< find and return upgrade by name
-	const UpgradeTemplate *findVeterancyUpgrade(VeterancyLevel level) const;				///< find and return upgrade by name
+	UpgradeTemplate *firstUpgradeTemplate( void ); ///< return the first upgrade template
+	const UpgradeTemplate *findUpgradeByKey( NameKeyType key ) const; ///< find upgrade by name key
+	const UpgradeTemplate *findUpgrade( const AsciiString& name ) const; ///< find and return upgrade by name
+	const UpgradeTemplate *findVeterancyUpgrade(VeterancyLevel level) const; ///< find and return upgrade by veterancy level
 
 	UpgradeTemplate *newUpgrade( const AsciiString& name );				///< allocate, link, and return new upgrade
 

GeneralsMD/Code/GameEngine/Include/GameClient/Image.h

@@ -128,7 +128,7 @@ class ImageCollection : public SubsystemInterface
 
 	void load( Int textureSize );												 ///< load images
 
-	const Image *findImageByName( const AsciiString& name );					 ///< find image based on name
+	const Image *findImageByName( const AsciiString& name ) const; ///< find image based on name
 
   /// adds the given image to the collection, transfers ownership to this object
   void addImage(Image *image);

GeneralsMD/Code/GameEngine/Source/Common/NameKeyGenerator.cpp

@@ -88,7 +88,7 @@ void NameKeyGenerator::freeSockets()
 
 }
 
-/* ------------------------------------------------------------------------ */
+//-------------------------------------------------------------------------------------------------
 inline UnsignedInt calcHashForString(const char* p)
 {
 	UnsignedInt result = 0;
@@ -98,7 +98,7 @@ inline UnsignedInt calcHashForString(const char* p)
 	return result;
 }
 
-/* ------------------------------------------------------------------------ */
+//-------------------------------------------------------------------------------------------------
 inline UnsignedInt calcHashForLowercaseString(const char* p)
 {
 	UnsignedInt result = 0;
@@ -164,71 +164,45 @@ NameKeyType NameKeyGenerator::nameToLowercaseKey(const char *name)
 }
 
 //-------------------------------------------------------------------------------------------------
-NameKeyType NameKeyGenerator::nameToKeyImpl(const char* nameString)
+NameKeyType NameKeyGenerator::nameToKeyImpl(const char* name)
 {
-	Bucket *b;
-
-	UnsignedInt hash = calcHashForString(nameString) % SOCKET_COUNT;
+	const UnsignedInt hash = calcHashForString(name) % SOCKET_COUNT;
 
-	// hmm, do we have it already?
+	// do we have it already?
+	const Bucket *b;
 	for (b = m_sockets[hash]; b; b = b->m_nextInSocket)
 	{
-		if (strcmp(nameString, b->m_nameString.str()) == 0)
+		if (strcmp(name, b->m_nameString.str()) == 0)
 			return b->m_key;
 	}
 
 	// nope, guess not. let's allocate it.
-	b = newInstance(Bucket);
-	b->m_key = (NameKeyType)m_nextID++;
-	b->m_nameString = nameString;
-	b->m_nextInSocket = m_sockets[hash];
-	m_sockets[hash] = b;
-
-	NameKeyType result = b->m_key;
-
-#if defined(RTS_DEBUG)
-	// reality-check to be sure our hasher isn't going bad.
-	const Int maxThresh = 3;
-	Int numOverThresh = 0;
-	for (Int i = 0; i < SOCKET_COUNT; ++i)
-	{
-		Int numInThisSocket = 0;
-		for (b = m_sockets[i]; b; b = b->m_nextInSocket)
-			++numInThisSocket;
-
-		if (numInThisSocket > maxThresh)
-			++numOverThresh;
-	}
-
-	// if more than a small percent of the sockets are getting deep, probably want to increase the socket count.
-	if (numOverThresh > SOCKET_COUNT/20)
-	{
-		DEBUG_CRASH(("hmm, might need to increase the number of bucket-sockets for NameKeyGenerator (numOverThresh %d = %f%%)",numOverThresh,(Real)numOverThresh/(Real)(SOCKET_COUNT/20)));
-	}
-#endif
-
-	return result;
-
+	return createNameKey(hash, name);
 }
 
 //-------------------------------------------------------------------------------------------------
-NameKeyType NameKeyGenerator::nameToLowercaseKeyImpl(const char* nameString)
+NameKeyType NameKeyGenerator::nameToLowercaseKeyImpl(const char* name)
 {
-	Bucket *b;
+	const UnsignedInt hash = calcHashForLowercaseString(name) % SOCKET_COUNT;
 
-	UnsignedInt hash = calcHashForLowercaseString(nameString) % SOCKET_COUNT;
-
-	// hmm, do we have it already?
+	// do we have it already?
+	const Bucket *b;
 	for (b = m_sockets[hash]; b; b = b->m_nextInSocket)
 	{
-		if (_stricmp(nameString, b->m_nameString.str()) == 0)
+		if (_stricmp(name, b->m_nameString.str()) == 0)
 			return b->m_key;
 	}
 
 	// nope, guess not. let's allocate it.
-	b = newInstance(Bucket);
+	return createNameKey(hash, name);
+}
+
+//-------------------------------------------------------------------------------------------------
+NameKeyType NameKeyGenerator::createNameKey(UnsignedInt hash, const AsciiString& name)
+{
+	Bucket *b = newInstance(Bucket);
 	b->m_key = (NameKeyType)m_nextID++;
-	b->m_nameString = nameString;
+	b->m_nameString = name;
 	b->m_nextInSocket = m_sockets[hash];
 	m_sockets[hash] = b;
 
@@ -256,7 +230,6 @@ NameKeyType NameKeyGenerator::nameToLowercaseKeyImpl(const char* nameString)
 #endif
 
 	return result;
-
 }
 
 //-------------------------------------------------------------------------------------------------

GeneralsMD/Code/GameEngine/Source/Common/System/Upgrade.cpp

@@ -293,7 +293,7 @@ void UpgradeCenter::reset( void )
 }
 
 //-------------------------------------------------------------------------------------------------
-/** Find upgrade matching name key */
+/** Find upgrade by veterancy level */
 //-------------------------------------------------------------------------------------------------
 const UpgradeTemplate *UpgradeCenter::findVeterancyUpgrade( VeterancyLevel level ) const
 {
@@ -302,7 +302,7 @@ const UpgradeTemplate *UpgradeCenter::findVeterancyUpgrade( VeterancyLevel level
 }
 
 //-------------------------------------------------------------------------------------------------
-/** Find upgrade matching name key */
+/** Find upgrade by name key */
 //-------------------------------------------------------------------------------------------------
 UpgradeTemplate *UpgradeCenter::findNonConstUpgradeByKey( NameKeyType key )
 {
@@ -329,7 +329,7 @@ UpgradeTemplate *UpgradeCenter::firstUpgradeTemplate( void )
 }
 
 //-------------------------------------------------------------------------------------------------
-/** Find upgrade matching name key */
+/** Find upgrade by name key */
 //-------------------------------------------------------------------------------------------------
 const UpgradeTemplate *UpgradeCenter::findUpgradeByKey( NameKeyType key ) const
 {
@@ -345,13 +345,11 @@ const UpgradeTemplate *UpgradeCenter::findUpgradeByKey( NameKeyType key ) const
 }
 
 //-------------------------------------------------------------------------------------------------
-/** Find upgrade matching name */
+/** Find upgrade by name */
 //-------------------------------------------------------------------------------------------------
 const UpgradeTemplate *UpgradeCenter::findUpgrade( const AsciiString& name ) const
 {
-
 	return findUpgradeByKey( TheNameKeyGenerator->nameToKey( name ) );
-
 }
 
 //-------------------------------------------------------------------------------------------------