Skip to content

Commit

Permalink
Add a string::ILess functor for case-insensitive containers
Browse files Browse the repository at this point in the history
Replace about 8 separate implementations of a std::less functor using
string_compare_nocase() with a single functor defined in string.h itself.

This is covered by tests in a new BasicTest suite which can be used for testing
simple core functions which do not require the construction of a complete
Radiant environment.
  • Loading branch information
Matthew Mott committed Mar 3, 2021
1 parent acc606c commit 258ad42
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 89 deletions.
14 changes: 14 additions & 0 deletions libs/string/string.h
Expand Up @@ -82,3 +82,17 @@ inline bool string_less_nocase(const char* string, const char* other)
{
return string_compare_nocase(string, other) < 0;
}

namespace string
{

/// Case-insensitive comparison functor for use with data structures
struct ILess
{
bool operator() (const std::string& lhs, const std::string& rhs) const
{
return string_compare_nocase(lhs.c_str(), rhs.c_str()) < 0;
}
};

}
10 changes: 1 addition & 9 deletions radiant/ui/einspector/EntityInspector.h
Expand Up @@ -66,14 +66,6 @@ class EntityInspector :
};

private:
struct StringCompareFunctorNoCase
{
bool operator()(const std::string& s1, const std::string& s2) const
{
// return boost::algorithm::ilexicographical_compare(s1, s2); // slow!
return string_compare_nocase(s1.c_str(), s2.c_str()) < 0;
}
};

// Currently selected entity, this pointer is only non-NULL if the
// current entity selection includes exactly 1 entity.
Expand Down Expand Up @@ -101,7 +93,7 @@ class EntityInspector :

// Cache of wxDataViewItems pointing to keyvalue rows,
// so we can quickly find existing keys to change their values
typedef std::map<std::string, wxDataViewItem, StringCompareFunctorNoCase> TreeIterMap;
typedef std::map<std::string, wxDataViewItem, string::ILess> TreeIterMap;
TreeIterMap _keyValueIterMap;

// Key and value edit boxes. These remain available even for multiple entity
Expand Down
13 changes: 2 additions & 11 deletions radiant/ui/mediabrowser/MediaBrowserTreeView.cpp
Expand Up @@ -39,15 +39,6 @@ namespace
}

/* Callback functor for processing shader names */
struct ShaderNameCompareFunctor
{
bool operator()(const std::string& s1, const std::string& s2) const
{
// return boost::algorithm::ilexicographical_compare(s1, s2); // slow!
return string_compare_nocase(s1.c_str(), s2.c_str()) < 0;
}
};

struct ShaderNameFunctor
{
// TreeStore to populate
Expand All @@ -60,7 +51,7 @@ struct ShaderNameFunctor

// Maps of names to corresponding treemodel items, for both intermediate
// paths and explicitly presented paths
using NamedIterMap = std::map<std::string, wxDataViewItem, ShaderNameCompareFunctor>;
using NamedIterMap = std::map<std::string, wxDataViewItem, string::ILess>;
NamedIterMap _iters;

wxIcon _folderIcon;
Expand Down Expand Up @@ -253,7 +244,7 @@ const MediaBrowserTreeView::TreeColumns& MediaBrowserTreeView::Columns() const
void MediaBrowserTreeView::SetTreeMode(MediaBrowserTreeView::TreeMode mode)
{
std::string previouslySelectedItem = GetSelectedFullname();

ResourceTreeView::SetTreeMode(mode);

// Try to select the same item we had as before
Expand Down
21 changes: 0 additions & 21 deletions radiantcore/commandsystem/CaseInsensitiveCompare.h

This file was deleted.

5 changes: 3 additions & 2 deletions radiantcore/commandsystem/CommandSystem.h
Expand Up @@ -2,17 +2,18 @@

#include "icommandsystem.h"
#include <map>
#include "CaseInsensitiveCompare.h"
#include "Executable.h"

#include "string/string.h"

namespace cmd
{

class CommandSystem :
public ICommandSystem
{
// The named executables (case-insensitive lookup)
typedef std::map<std::string, ExecutablePtr, CaseInsensitiveCompare> CommandMap;
typedef std::map<std::string, ExecutablePtr, string::ILess> CommandMap;
CommandMap _commands;

public:
Expand Down
11 changes: 1 addition & 10 deletions radiantcore/eclass/EntityClass.h
Expand Up @@ -33,15 +33,6 @@ class EntityClass
private:
typedef std::shared_ptr<std::string> StringPtr;

class StringCompareFunctor
{
public:
bool operator()(const std::string& lhs, const std::string& rhs) const
{
return string_compare_nocase(lhs.c_str(), rhs.c_str()) < 0;
}
};

// The name of this entity class
std::string _name;

Expand Down Expand Up @@ -72,7 +63,7 @@ class EntityClass

// Map of named EntityAttribute structures. EntityAttributes are picked
// up from the DEF file during parsing. Ignores key case.
typedef std::map<std::string, EntityClassAttribute, StringCompareFunctor> EntityAttributeMap;
typedef std::map<std::string, EntityClassAttribute, string::ILess> EntityAttributeMap;
EntityAttributeMap _attributes;

// The model and skin for this entity class (if it has one)
Expand Down
14 changes: 1 addition & 13 deletions radiantcore/entity/KeyObserverMap.h
Expand Up @@ -31,24 +31,12 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
namespace entity
{

/**
* Comaparator to allow for case-insensitive search in std::multimap
*/
struct CaseInsensitiveKeyCompare
{
bool operator()(const std::string &s1, const std::string &s2) const
{
// return boost::algorithm::ilexicographical_compare(s1, s2); // slow!
return string_compare_nocase(s1.c_str(), s2.c_str()) < 0;
}
};

class KeyObserverMap :
public Entity::Observer,
public sigc::trackable
{
// A map using case-insensitive comparison
typedef std::multimap<std::string, KeyObserver*, CaseInsensitiveKeyCompare> KeyObservers;
typedef std::multimap<std::string, KeyObserver*, string::ILess> KeyObservers;
KeyObservers _keyObservers;

// The observed entity
Expand Down
5 changes: 3 additions & 2 deletions radiantcore/shaders/ShaderDefinition.h
Expand Up @@ -5,7 +5,8 @@
#include <map>
#include <string>
#include "ShaderTemplate.h"
#include "ShaderNameCompareFunctor.h"

#include "string/string.h"

namespace shaders
{
Expand All @@ -31,6 +32,6 @@ struct ShaderDefinition

};

typedef std::map<std::string, ShaderDefinition, ShaderNameCompareFunctor> ShaderDefinitionMap;
typedef std::map<std::string, ShaderDefinition, string::ILess> ShaderDefinitionMap;

}
8 changes: 4 additions & 4 deletions radiantcore/shaders/ShaderLibrary.h
Expand Up @@ -5,7 +5,7 @@
#include "CShader.h"
#include "TableDefinition.h"

namespace shaders
namespace shaders
{

class ShaderLibrary
Expand All @@ -14,11 +14,11 @@ class ShaderLibrary
// These are referenced by name.
ShaderDefinitionMap _definitions;

typedef std::map<std::string, CShaderPtr, ShaderNameCompareFunctor> ShaderMap;
typedef std::map<std::string, CShaderPtr, string::ILess> ShaderMap;
ShaderMap _shaders;

// The lookup tables used in shader expressions
typedef std::map<std::string, TableDefinitionPtr, ShaderNameCompareFunctor> TableDefinitions;
typedef std::map<std::string, TableDefinitionPtr, string::ILess> TableDefinitions;
TableDefinitions _tables;

public:
Expand All @@ -33,7 +33,7 @@ class ShaderLibrary
*/
ShaderDefinition& getDefinition(const std::string& name);

/**
/**
* Returns true if the given shader definition exists.
*/
bool definitionExists(const std::string& name) const;
Expand Down
17 changes: 0 additions & 17 deletions radiantcore/shaders/ShaderNameCompareFunctor.h

This file was deleted.

35 changes: 35 additions & 0 deletions test/Basic.cpp
@@ -0,0 +1,35 @@
/**
* Tests for basic library functions (such as string comparison) which do not
* require an actual Radiant environment.
*/

#include "gtest/gtest.h"

#include "string/string.h"

namespace test
{

TEST(BasicTest, StringCompareNoCase)
{
EXPECT_EQ(string_compare_nocase("blah", "blah"), 0);
EXPECT_EQ(string_compare_nocase("blah", "BLAH"), 0);
EXPECT_EQ(string_compare_nocase("MiXeD", "mIxED"), 0);

EXPECT_EQ(string_compare_nocase("a", "b"), -1);
EXPECT_EQ(string_compare_nocase("b", "a"), 1);
EXPECT_EQ(string_compare_nocase("baaaaa", "aaaaa"), 1);
}

TEST(BasicTest, StringILessFunctor)
{
string::ILess less;

EXPECT_TRUE(!less("blah", "BLAH"));
EXPECT_TRUE(!less("BLAH", "blah"));

EXPECT_TRUE(less("blah", "BLEH"));
EXPECT_TRUE(!less("BLEH", "blah"));
}

}
1 change: 1 addition & 0 deletions test/CMakeLists.txt
@@ -1,4 +1,5 @@
add_executable(drtest
Basic.cpp
Camera.cpp
ColourSchemes.cpp
CSG.cpp
Expand Down

0 comments on commit 258ad42

Please sign in to comment.