Reputation: 5931
I have a class complicated
which features various setters that modify some internal state. That internal state modification is potentially expensive, so I want to do it not too often. In particular, if several setters are invoked in immediate succession, I want to perform the expensive update of the internal state only once after the last of these setter invocations.
I have solved (or "solved"?) that requirement with a proxy. The following would be a minimal working code example:
#include <iostream>
class complicated
{
public:
class proxy
{
public:
proxy(complicated& tbu) : to_be_updated(&tbu) {
}
~proxy() {
if (nullptr != to_be_updated) {
to_be_updated->update_internal_state();
}
}
// If the user uses this operator, disable update-call in the destructor!
complicated* operator->() {
auto* ret = to_be_updated;
to_be_updated = nullptr;
return ret;
}
private:
complicated* to_be_updated;
};
public:
proxy set_a(int value) {
std::cout << "set_a" << std::endl;
a = value;
return proxy(*this);
}
proxy set_b(int value) {
std::cout << "set_b" << std::endl;
b = value;
return proxy(*this);
}
proxy set_c(int value) {
std::cout << "set_c" << std::endl;
c = value;
return proxy(*this);
}
void update_internal_state() {
std::cout << "update" << std::endl;
expensive_to_compute_internal_state = a + b + c;
}
private:
int a;
int b;
int c;
int expensive_to_compute_internal_state;
};
int main()
{
complicated x;
x.set_a(1);
std::cout << std::endl;
x.set_a(1)->set_b(2);
std::cout << std::endl;
x.set_a(1)->set_b(2)->set_c(3);
}
It produces the following output which is looking like exactly what I wanted:
set_a
updateset_a
set_b
updateset_a
set_b
set_c
update
My questions are: Is my approach legit/best practice?
Is it okay to rely on temporary objects (i.e. the proxy
objects which are returned) which will be destroyed at the semicolon?
I'm asking because I have a bad feeling about this for some reason. Maybe my bad feeling comes just from Visual Studio's warning which says:
Warning C26444 Avoid unnamed objects with custom construction and destruction (es.84).
But maybe/hopefully my bad feelings are unjustified and that warning can just be ignored?
What bothers me the most: Is there any case in which the update_internal_state
method will NOT be called (maybe by misusing my class or by some compiler optimization or whatever)?
Lastly: Is there any better approach to implement what I try to achieve with modern C++?
Upvotes: 3
Views: 3312
Reputation: 25516
Following Dmitry Gordon's argument of people selecting 'wrong' approach, you might have the matter a bit simpler, though (especially from user's view):
class Demo
{
int m_x
int m_y; // cached value, result of complex calculation
bool m_isDirtyY;
public:
int x() { return m_x; }
void x(int value) { m_x = value; m_isDirtyY = true; }
int y()
{
if(m_isDirtyY)
{
// complex calculations...
m_y = result;
m_isDirtyY = false;
}
return m_y;
}
};
This way, you'll only ever execute the calculations on need, no additional extensions like the proxy objects or explicit transactions.
Depending on your needs, you might perhaps encapsulate this pattern in a separate (template?) class, perhaps receiving an updater object (lambda?) or with a pure virtual update
function to repeat less code.
Side note: Setting a value might invalidate more than one cached value – no problem, set more than one dirty flag to true then...
Upvotes: 0
Reputation: 23681
The danger I see here is if your class has any methods that do not return a proxy (or any public members). You disable the update call if operator->
of the proxy is used (which yields the complicated
), but this is only safe if that usage of operator->
always yields another proxy object which will take over the updating task. This seems like a huge pitfall for anybody who modifies the class later on.
I think it would be safer if complicated
were to keep track of the number of alive proxy
objects created on it so that the last proxy
to be destroyed performs the update call.
Upvotes: 2
Reputation: 2324
I think your solution is legit, but it has a drawback that it hides from the user of the code, that the update is expensive, so one will more likely write:
x.set_a(1);
x.set_b(2);
than
x.set_a(1)->set_b(2);
I would suggest make setters private and add a friend transaction class, so that modifying object would look like:
complicated x;
{
transaction t(x);
t.set_a(1);
t.set_b(2);
// implicit commit may be also done in destructor
t.commit();
}
If transaction
will be the only way to modify complicated
- users will more tend to call several setters in a one transaction.
Upvotes: 6