CODEWITHSUNDEEP
c++if-statementsfml
de_fault
de_fault

Reputation: 76

Is there a more elegant way to use one else statement with independent if()s

I'm currently working in SFML for c++ and trying to resize a window however the solution I found to my problem doesn't quite feel right and I'm looking if there is a better way.

I have an object that gets resized multiple times so it may fit inside the window or not, I have to expand the window if its bigger and shrink it if its smaller. However I have a minimum size for the window, if the object fits inside that I want to reset the window to that size.

This is the pseudo-code I'm using right now:

if (Object.getSize().x > window.getSize().x || Object.getSize().y > window.getSize.y){
    if(Object.getSize().x > windowMinSize.x){
        window.resize(Object.getSize().x, window.getSize().y)
    }
    if(Object.getSize().y > windowMinSize.y){
        window.resize(window.getSize().x, Object.getSize().y)
    }
}
else{
    window.resize(windowMinSize);
}

I've looked into switch and other options but I haven't found what I'm looking for.

Upvotes: 1

Views: 138

Answers (4)

M.M
M.M

Reputation: 141628

I am interpreting your question as asking how to refactor the following code:

if ( A )
{
     if ( B ) { X } else { Y }
     if ( C ) { Z } else { Y }
}
else { Y }

to avoid repetition of Y.

One way would be:

bool A = ....;
bool B = ....;
bool C = ....;

if      ( A && B ) { X }
else if ( A && C ) { Z }
else { Y }

although this may involve unnecessary function calls compared to short circuiting behaviour of the original code.

The most "obvious" solution is storing a variable:

bool updated = false;
if ( A ) 
{
    if ( B ) { X; updated = true; }
    if ( C ) { Z; updated = true; }
}
if ( !updated ) { Y }

Another way would be to use a control structure you can break out of:

do
{
     if ( A )
     {
          if ( B ) { X; break; }
          if ( C ) { Z; break; }
     }
     Y;
} while (0);

Some people dislike this because if you are reading the code it looks like we are entering a loop, but it later turns out not to be a loop. Personally I would make a function (with break being replaced by return); but if using the do...while(0) you should put a code comment at the start to point out that this is not really a loop.

Upvotes: 1

Mario
Mario

Reputation: 36507

I'm a bit confused, because in the end it sounds like you want to simply resize your window to the object size while keeping a specific minimum size.

As such all you'd need is a simple call to std::max() to determine the bigger value of two inputs:

unsigned int width = std::max(min_width, object_width);
unsigned int height = std::max(min_height, object_height);

When using the new size it will basically resize the window to match your object unless the object is smaller than your minimum size.

You can use the same pattern to also add a maximum size using std::min() as an additional layer:

unsigned int width = std::min(max_width, std::max(min_width, object_width));
unsigned int height = std::min(max_height, std::max(min_height, object_height));

Edit: As bolov correctly suggested, this can be simplified even more for modern compilers using the new std::clamp:

unsigned int width = std::clamp(object_width, min_width, max_width);
unsigned int height = std::clamp(object_height, min_height, max_height);

Upvotes: 1

Stephen Newell
Stephen Newell

Reputation: 7863

You could play with variadic templates to get what you want. My version here takes an else function (the first argument), then pairs of functions (predicate, block, predicate, block, ...). If none of the predicates evaluate to true, the else function will be executed. This should work with C++11 or later.

There may be some bugs here (I haven't done much testing), but you can kick the tires. If you change the #if 0 to #if 1 you can observe that the else function won't be called.

#include <iostream>

template <typename PRED, typename EXEC>
bool multi_branch_detail(PRED pred, EXEC exec) {
    if(pred()) {
        exec();
        return true;
    }
    return false;
}

template <typename PRED, typename EXEC, typename ...REM>
bool multi_branch_detail(PRED pred, EXEC exec, REM ...rem) {
    auto result = false;
    if(pred()) {
        exec();
        result = true;
    }
    return multi_branch_detail(std::forward<REM>(rem)...) || result;
}

template <typename ELSE, typename ...FNS>
void multi_branch(ELSE el, FNS ...fns) {
    if(!multi_branch_detail(std::forward<FNS>(fns)...)) {
        el();
    }
}


int main() {
    multi_branch(
        []() { std::cout << "No cases\n"; },

#if 0
        []() { return 1 < 2; }, []() { std::cout << "first case\n"; },
        []() { return 10 < 20; }, []() { std::cout << "second case\n"; },
#endif
        []() { return 1 > 2; }, []() { std::cout << "bug\n"; }
    );
    return 0;
}

Output with #if 0:

No cases

Output with #if 1:

first case
second case

Upvotes: 1

user4442671
user4442671

Reputation:

The right pattern to be using here is to limit the number of code paths that invoke the external method so that there is a single point of usage:

Your pseudo-code becomes something like this:

auto new_win_size = windowMinSize;
if (Object.getSize().x > window.getSize().x || Object.getSize().y > window.getSize.y){
    if(Object.getSize().x > windowMinSize.x){
        new_win_size.x = Object.getSize().x;
        new_win_size.y = window.getSize().y;
    }
    if(Object.getSize().y > windowMinSize.y){
        new_win_size.x = window.getSize().x;
        new_win_size.y = Object.getSize().y;
    }
}

window.resize(new_win_size );

But I suspect what you actually want is:

auto new_win_size = windowMinSize;

if(Object.getSize().x > windowMinSize.x){
    new_win_size.x = Object.getSize().x;
}
if(Object.getSize().y > windowMinSize.y){
    new_win_size.y = Object.getSize().y;
}

window.resize(new_win_size );

Upvotes: 1

Related Questions