Check for changes in POD variables

Question

I'm looking for an efficient way to check if a POD variable is altered between two cycles. I've come up with this solution:

class Foo {
public:
    template<typename T>
    bool isChanged(T& entry);
    void endCycle();
private:
     std::map<void*,size_t> entryMap;  // <Address orig.,Size>
     std::map<void*,void*>oldVals;     // <Address orig., Address cpy.> 
};

template<typename T> bool Foo::isChanged(T& entry)
{
    entryMap[&entry] = sizeof(T);
    if(oldVals[&entry] == NULL)
        return false;
    if(memcmp(&entry, oldVals[&entry], entryMap[&entry]))
        return true;
    else
        return false;
}

void Foo::endCycle()
{   
    // Copy all the bytes to save them for the next cycle
    for(  std::map<void*,size_t>::iterator entryIt = entryMap.begin();
          entryIt != entryMap.end();
          ++entryIt)
    {
        if(oldVals[entryIt->first] == NULL)
            oldVals[entryIt->first] = malloc(entryIt->second);
        memcpy(oldVals[entryIt->first], entryIt->first, entryIt->second);
    }
}

Now i can use it like this:

Foo gBar;
void aFunction()
{
int ar;
char ba[3][3];
// Some code where ar and ba are filled
if(gBar.isChanged(ar))
    // Do Something
if(gBar.isChanged(ba))
    // Do Something
gBar.endCycle();
}

Is this an efficient way? My goal was a method which is very easy to use inside various cyclically called functions. I cleaned all the init and free logic from the code. Any suggestions? I especially don't like the oldshool malloc, memcpy and memcmp stuff but i don't know any other way how to do it.

Edit: Found a good solution based on Red Alerts suggestions.

Answer 1

I think you can use templates a little more effectively here.

template <typename T>
class Foo
{
public:
    static std::map<T*, T> values;

    static bool isChanged(T& entry)
    {    
        auto it = values.find(&entry);

        if(it == values.end())
        {
            values[&entry] = entry;
        }
        else if(entry != it->second)
        {    
            it->second = entry;
            return true;
        }

        return false;
    }
};


template <typename T>
std::map<T*, T> Foo<T>::values;

int main() {

    int ar = 3;
    cout << Foo<int>::isChanged(ar) << endl; // 0

    ar = 4;

    cout << Foo<int>::isChanged(ar) << endl; // 1

    for(auto& value : Foo<int>::values)
        cout << value.second << endl; // 4

    return 0;
}

This way you get one map per type, and you don't have to worry about inadvertently messing up an alias. You do need to define operator != and have a working copy constructor for your types, but that is much better than blindly using memcmp and memcpy.

You can also make further template specializations for arrays if you need to compare those (will be a bit more code, but nothing very complicated)

Edit: To get you started, this is what your template signature should look like:

template<class T, size_t N> bool isChanged(T(&entry)[N]); //will be called for stack allocated arrays

Or you can use char* to alias all of your values. This will let you use a single map for everything (like you were doing before, but this has no memcpy/memcmp). It will only work for POD. We could manually call the destructor when overwriting the buffer, but since there is no good way to do this in the class's destructor, it's probably best to leave out heap allocated data altogether.

class Foo
{
    std::map<char**, char*> values;

public:
    ~Foo()
    {
        for(auto& value : values)
        {
            delete[] value.second;
        }
    }

    template<typename T> bool isChanged(T& entry)
    {
        char** addr = reinterpret_cast<char**>(&entry);
        auto it = values.find(addr);

        if(it == values.end())
        {
            alignas(T) char* oldBuf = new char[sizeof(T)];
            T* oldEntry = new(oldBuf) T;
            *oldEntry = entry;
            values[addr] = oldBuf;
        }
        else if(entry != *(reinterpret_cast<T*>(it->second)))
        {
            T* oldEntry = new(it->second) T;
            *oldEntry = entry;
            return true;
        }

        return false;
    }
};

Answer 2

After many hours i think i found a good solution. The call stays easy and there are no casts. It's a lot more complex than the C-Style version with memcopy but I think its nicer and has also the benefit that it works with complex data not just POD.

class Manager
{
public:
    ~Manager()
    {
        funcPtrs.clear();
    }
    void adFnc(void(*function)())
    {
        funcPtrs.push_back(function);
    }
    void runAll()
    {
        for(auto& val : funcPtrs)
            val();
    }
private:
    std::vector<void (*)()> funcPtrs;
};

Manager gAllClearManager;

template<typename T>
class Data
{
public:
    Data()
    {
        gAllClearManager.adFnc(clearValues);
    }
    static void clearValues()
    {
        values.clear();
    }
    static std::map<T*,std::vector<T>>& getValues() { return values; }
private:
    static std::map<T*,std::vector<T>> values;
};

template <typename T>
static bool isChanged(T& entry)
{
    const static Data<T>* dataP = new  Data<T>();
    static std::map<T*,std::vector<T>>&  values = dataP->getValues();

    auto it = values.find(&entry);

    if(it == values.end())
    {
        values[&entry].push_back(entry);
    }
    else if(entry != it->second[0])
    {    
        it->second[0] = entry;
        return true;
    }
    return false;
}

template<typename T, size_t N>
bool isChanged(T (&entry)[N])
{
    const static Data<T>* dataP = new  Data<T>();
    static std::map<T*,std::vector<T>>&  values = dataP->getValues();

    auto it = values.find(entry);

    if( it == values.end())
    {
        for(int i = 0; i < N ; ++i )
            values[entry].push_back(entry[i]);
        return false;
    }
    else
    {
        for(int i = 0; i < N ; ++i )
        {
            if(it->second[i] != entry[i])
            {
                for(int j = 0; j < N ; ++j )
                {
                    it->second[j] = entry[j];
                }
                return true;
            }
        }
    }
    return false;
}

template<typename T>
std::map<T*, std::vector<T>> Data<T>::values;

Now i can use it like:

int main() {
    int ar;
    std::string ba[6];
    if(isChange(ar))
        // Do something
    if(isChange(ba)) 
        // Do something
}

My first template is finally working! :) Thanks again Red Alert.