0

My system defines multiple unordered_map-like data, e.g.

 std::unordered_map< int, int> int_map; std::unordered_map< int, double> double_map; std::unordered_map< string, string> string_map; std::unordered_map< string, string> string_map_2; ... std::unordered_map< string, vector<string> > string_string_map; std::unordered_map< string, vector<string> > string_string_map_2;

Each map holds different data types. These are used with a framework (written in C) that registers callback functions. The registration function (register_undo_handler) takes two arguments: a pointer to the callback function and data to be passed to the callback. In our case, the data is a map pointer.

 //register our callback function to UNDO framework register_undo_handler(clear_data, &int_map); //or : register_undo_handler(clear_data, &string_map); // this is a sketch of the callback function, which frees the map resources void clear_data(void *data) { data->clear(); }

If an UNDO happens, the UNDO framework will call each defined callback function, e.g. clear_data, and pass the registered data, cast to void*, as the parameter like this

 // For each registered callback function, run with registered parameter (*registered_callback_func)(data);

We want to have a single callback function that can free any map. Any suggestion on the implementation?

Improve this question
8
  • 3
    You can just make undo_clear_data a function template, but why don't you just call data->clear() directly? Commented Sep 17, 2020 at 15:16
  • 1
    an unorder_map takes ordered input and scrambles it? Serisouly, the template is called std::unordered_map. Please show real code Commented Sep 17, 2020 at 15:16
  • void undo_clear_data(void *data) { data->clear(); } hmmm... Commented Sep 17, 2020 at 15:22
  • thanks for the comment, i alreayd make it more clear. hope this will be helpful Commented Sep 17, 2020 at 15:37
  • @zheng what's the signature of register_undo_handler? Commented Sep 17, 2020 at 15:39

3 Answers 3

Reset to default
3

you can have single template to do it if you just don't want to dupe code.

#include <functional> #include <unordered_map> #include <iostream> using namespace std; void register_undo_handler(void(*callback)(void*), void* data){ callback(data); //I'd not gonna implement all undo in C so let me do this instead } template<typename clearable> void Erase(void* data){ ((clearable*)data)->clear(); } int main(){ std::unordered_map<int, int> int_map; std::unordered_map<int, double> double_map; double_map[0] = int_map[0] = 0; //you can wrap this into a function to avoid type types yourself register_undo_handler(&Erase<decltype(int_map)>, (void*)&int_map); register_undo_handler(&Erase<decltype(double_map)>, (void*)&double_map); std::cout << int_map.size() << ' ' << double_map.size(); }
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

0

When an "undo" callback is invoked, there are two pieces of information available, namely the address of the function to call and the pointer-sized data to provide to the function. Your requirement is for the first piece to not vary, so it is up to the second piece to tell us what type of map is involved. A big problem is that once a pointer is cast to void*, there is no longer a way to recover its original type. So to accomplish the stated goal, you would need a way to identify the map with something other than a raw pointer. One possibility is wrapping the pointer in an object that could remember the original type. This can get messy, especially if you need to dynamically allocate these wrapper objects. The overhead for coding this is more than for writing multiple callback functions. Similarly, there is no savings in runtime overhead (neither time nor space). This is probably not what you want to do.

It is much simpler to write a callback for each type. It takes only a few lines of code to define the callbacks. Each function compiles into a small amount of object code; taken together, the compiled functions are almost the theoretical minimum amount of object code required for any solution. (Each map type's clear function is a distinct function, so it takes distinct function calls to potentially call each of them. I'm above the minimum because I check for null.)

template <class T> void call_clear(void * ptr) { if ( ptr ) static_cast<T*>(ptr)->clear(); }

Registering one of these callbacks is a bit of a syntactic pain (e.g. register_undo_handler( call_clear<decltype(int_map)>, &int_map);), so it might be desirable to write a wrapper to handle the registration. This wrapper can deduce the template parameter, reducing the amount of repetitive typing. The compiler should inline this function at any level of optimization, resulting in no additional runtime overhead.

template <class T> void register_undo_call_clear(T & map) { register_undo_handler(call_clear<T>, &map); }

At this point, registering the callback is done simply with, for example, register_undo_call_clear(int_map);.

Improve this answer

Comments

0

EDIT : I just realize it's not easier than write every callback function (which still needs to be done) if it's only to clear those map... I'd leave this here as a reference for general-propose handler to C handler solution. (btw technically it does only need one callback function(1st parameter) 😑 )

First, I'd strongly suggest move to a c++ undo framework

But you can actually archive this with single handler (1st parameter).

The point is use a type-erased handler (std::function at below code) passed as data (2nd parameter), and invoke it at the wrapper handler (1st parameter)

note: you still not adding the signature of register_undo_handler so I guess it's

void register_undo_handler(void(*)(void), void*);

The code ( Wandbox )

#include <functional> #include <list> #include <stack> #include <iostream> using namespace std; void register_undo_handler(void(*callback)(void*), void* data){ callback(data); //I'd not gonna implement all undo in C so let me do this instead } void the_callback_wrapper(void* cb){ (*static_cast<function<void()>*>(cb))(); } int main(){ list<function<void()>> undos; //and your maps, they should have same lifetime int a=2,b=3; //do something... undos.emplace_back([]{std::cout << "I'd like to undo this - 1\n";}); register_undo_handler(the_callback_wrapper, (void*)(&undos.back())); //undo with local variables, no problem undos.emplace_back([&]{std::cout << "I'd like to undo this - " << a << '\n';}); register_undo_handler(the_callback_wrapper, (void*)(&undos.back())); undos.emplace_back([&]{std::cout << "I'd like to undo this - " << b << '\n';}); register_undo_handler(the_callback_wrapper, (void*)(&undos.back())); //modify local variable, no problem undos.emplace_back([&]{std::cout << "I'd like to modify a to 100\n"; a=100;}); register_undo_handler(the_callback_wrapper, (void*)(&undos.back())); std::cout << "a should be 100 : a = " << a << '\n'; //use multiple local variable, no problem undos.emplace_back([&]{std::cout << "I'd like to modify **both a and b** to 10000\n"; a=b=100000;}); register_undo_handler(the_callback_wrapper, (void*)(&undos.back())); std::cout << a << ' ' << b; }

sidenote: If you can use global variable to store whose undo functors, you can also pass their index as data and let the callback decide which functor to call.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.