patterncppMinor
Simplifying logic of overlapping predicates
Viewed 0 times
predicateslogicoverlappingsimplifying
Problem
I have 3 simple predicates and 3 simple actions to be taken based on those predicates. In my actual application they are not based on integer arithmetic, and in fact are rather expensive to compute (in comparison to the actions taken), but their dependency chain is the same.
Note that
However, the following straightforward try at refactoring is a lot more concise but also less efficient as it computes all 3 predicates for all inputs:
So I came up with the idea of caching the predicate values in a
Live Example.
So the repetitive logic is hidden inside the repeated values of
#include
#include
bool pred1(int x) { return x % 30 == 0; }
bool pred2(int x) { return x % 15 == 0; }
bool pred3(int x) { return x % 5 == 0; }
void fun1(int x) { std::cout << "divisible by 30 \n"; }
void fun2(int x) { std::cout << "divisible by 15 \n"; }
void fun3(int x) { std::cout << "divisible by 5 \n"; }
void fun(int x)
{
if (pred1(x)) {
fun1(x);
fun2(x);
fun3(x);
return;
}
if (pred2(x)) {
fun2(x);
fun3(x);
return;
}
if (pred3(x))
fun3(x);
}Note that
pred1 requires all 3 actions, pred2 the last 2 actions, and pred3 only the 3rd action. I was not happy with this code: while efficient in avoiding extra work, it seems overly repetitive. However, the following straightforward try at refactoring is a lot more concise but also less efficient as it computes all 3 predicates for all inputs:
// exposition only: will compute pred1, pred2 and pred3 for all inputs
void hun(int x)
{
if (pred1(x)) fun1(x);
if (pred2(x)) fun2(x);
if (pred3(x)) fun3(x);
}So I came up with the idea of caching the predicate values in a
std::tuple and dispatch the various actions based on that:using Triple = std::tuple;
auto preds(int x) -> Triple
{
if (pred1(x)) return Triple{ true, true, true };
if (pred2(x)) return Triple{ false, true, true };
return Triple{ false, false, pred3(x) };
}
void gun(int x)
{
auto const p = preds(x);
if (std::get(p)) fun1(x);
if (std::get(p)) fun2(x);
if (std::get(p)) fun3(x);
}Live Example.
So the repetitive logic is hidden inside the repeated values of
true in the std::tuple, and the duplicate predicate Solution
It seems to me that the primary role of the programmer is to write clear and correct code and that it's largely the job of the compiler to turn that into efficient machine code. With that said, it seems to me that the first
Your
If you'd like to reduce the repetition, the way to do that would be to create subfunctions such as this:
Once this is done, the multiple
If the predicates and functions are dealing solely with integers it doesn't much matter, but if they are dealing with larger items, and particularly objects with constructors and destructors, it would be helpful to use references and
fun() version is both clear and effective in that it avoids the needless overhead of computing conditions that don't need to be computed, and is sufficiently clear about the results of each predicate. Your
gun() version separates the predicates from the actions using an intermediary Triple and while it produces the same result, it makes the job of interpreting the code (for a human) a little bit harder. So I would advocate not doing it that way.If you'd like to reduce the repetition, the way to do that would be to create subfunctions such as this:
void do123(int x) {
fun1(x);
fun2(x);
fun3(x);
}Once this is done, the multiple
if statements become a bit shorter and easier to read, although at some possible expense in clarity. If the predicates and functions are dealing solely with integers it doesn't much matter, but if they are dealing with larger items, and particularly objects with constructors and destructors, it would be helpful to use references and
const as much as possible to avoid useless object copies.Code Snippets
void do123(int x) {
fun1(x);
fun2(x);
fun3(x);
}Context
StackExchange Code Review Q#57477, answer score: 2
Revisions (0)
No revisions yet.