Object storing and retrieving using wildcard identifier

I'm writing an interpreter for a scripting language which allows objects to:

• Have an alias

• Be referenced using a wildcard

• Contain child objects

• Combination of all three above

For example:

CreateWindow("win", ...);
CreateName("win/texes1", ...);
CreateName("win/texes2", ...);
CreateAlias("win/texes1", "wintexes");
CreateAlias("win/texes2", "wintexes2");
CreateTexture("@wintexes/tex1", ...);
CreateTexture("@wintexes/tex2", ...);
Delete("@wintexes/*"); // Deletes textures
Delete("@wintexes*"); // Deletes names
Delete("win");

Crazy, I know. It wasn't me who came up with it though.

In order to call the function on all objects intended to be affected by it, I came up with this code:

```
class ObjectHolder_t : private Holder
{
public:
virtual ~ObjectHolder_t()
{
}

Object* Read(const string& Handle)
{
string Leftover = Handle;
string ObjHandle = ExtractObjHandle(Leftover);
if (Leftover.empty())
return Holder::Read(ObjHandle);
return GetHolder(ObjHandle)->Read(Leftover);
}

void Write(const string& Handle, Object* pObject)
{
string Leftover = Handle;
string ObjHandle = ExtractObjHandle(Leftover);
if (Leftover.empty())
Holder::Write(ObjHandle, pObject);
else
GetHolder(ObjHandle)->Write(Leftover, pObject);
}

template
void Execute(const string& Handle, F Func)
{
string Leftover = Handle;
string ObjHandle = ExtractObjHandle(Leftover);
if (ObjHandle.back() == '*')
ObjHandle.front() == '@' ? WildcardAlias(Leftover, ObjHandle, Func) : WildcardCache(Leftover, ObjHandle, Func);
else
Leftover.empty() ? Func(&Cache.find(ObjHandle)->second) : GetHolder(ObjHandle)->Execute(Leftover, Func);
}

void WriteAlias(const string& Handle, const string& Alias)
{
Aliases[Alias] = Handle;
}

private:
template
void ExecuteSafe(const string&

The code is a bit verbose, which makes it hard to review in a tiny StackOverflow window... but here are my thoughts. I'll start with the nitpicks and then see about bigger issues.

(1) You use unadorned string and map, which means you're relying on using namespace std;. Don't do that. It's Just Not Done in typical C++ code.

(2) ObjectHolder_t has a virtual destructor, which is nice, but it already inherits from Holder, whose destructor isn't virtual. (It's private inheritance, so I guess that's sort-of okay... but again it's bad style. It sends up red flags for the reader (me), which wastes my time even if they're false alarms.)

Also, speaking of the verbosity of this code, it's good style to put those empty braces on a single line instead of spread over three lines. Use virtual ~Holder() {} or virtual ~Holder() = default;, and remove the now-redundant destructor from ObjectHolder_t.

(3) std::map is a code smell; code using it can usually be optimized by replacing it with std::unordered_map. Unless of course your algorithm requires that the map be sorted, in which case, I'd add a // comment explaining that, right at the point where the reader is going to be smelling that std::map.

(4) ExtractObjHandle mutates its argument, in addition to returning another string by value. That's a big smelly code smell, and indeed we can see that the one place you use this subroutine (in Read, Write, and Execute), you have to explicitly copy the argument string because you don't want the original to be mutated. The proper way to return a pair of strings from a method is to change the return type to std::pair. At that point you can also consider changing the parameter type from const std::string& to std::string if it would eliminate an explicit copy operation.

(5) That MapDeleter cruft seems like a buggy reimplementation of std::unique_ptr: you're just looking for something that holds a pointer and deletes it when it's destroyed. You can safely make the substitution.

(6) In fact, this whole "cache" thing is a bit boned from the start, because the Read method allows a pointer to "escape" from the cache's map. The caller could store that pointer somewhere, and then the cache gets destroyed (via ~Holder), which destroys the pointed-to object... and you end up with undefined behavior the next time you try to access the object through the original pointer. To fix this, the textbook approach is to use std::shared_ptr to manage lifetimes, and the real-world approach is "don't do that then". I'll assume your code doesn't do that, and that returning raw pointers to managed objects is totally fine by you.

(7) Nitpick: constify your const methods.

(8) Another nitpick: prefer Handle.npos to std::string::npos; it's just one fewer thing to worry about whether you matched up the types correctly. Modern C++ is quickly moving in the direction of "generic programming", which in a nutshell means never having to explicitly write out your data types if you don't need to. See also Herb Sutter's concept of "Almost Always Auto".

(9) In Regexify, you return string(...); where ... represents an expression of type string. The correct idiom here is simply return ...;. The former means "Evaluate ..., move-construct another string from it, and then move-construct the return value from that string." (The last move-construction may be elided.) As opposed to return ...;, which means "Evaluate ..., and then move-construct the return value from it." (The last move-construction may be elided.)

So let's see, what are we left with...?

```
template struct Holder
{
T* Read(const std::string& Path) const {
auto iter = Cache.find(Path);
return iter == Cache.end() ? nullptr : iter->second.get();
}

void Write(const std::string& Path, T* Data) {
Cache[Path] = std::unique_ptr(Data);
}

virtual ~Holder() = default;

std::unordered_map> Cache;
};

class ObjectHolder_t : private Holder
{
public:
Object* Read(const std::string& Handle) {
std::string ObjHandle, Leftover;
std::tie(ObjHandle, Leftover) = ExtractObjHandle(Handle);
if (Leftover.empty())
return Holder::Read(ObjHandle);
return GetHolder(ObjHandle)->Read(Leftover);
}

void Write(const string& Handle, Object* pObject) {
std::string ObjHandle, Leftover;
std::tie(ObjHandle, Leftover) = ExtractObjHandle(Handle);
if (Leftover.empty())
Holder::Write(ObjHandle, pObject);
else
GetHolder(ObjHandle)->Write(Leftover, pObject);
}

template
void Execute(const string& Handle, F Func) {
std::string ObjHandle, Leftover;
std::tie(ObjHandle, Leftover) = ExtractObjHandle(Handle);
if (ObjHandle.back() == '*') {
ObjHandle.front() == '@' ? WildcardAlias(Leftover, ObjHandle, Func) : WildcardCache(Leftover, ObjHandle, Func);
} else {

StackExchange Code Review Q#71953, answer score: 3