Can I lessen the use of boolean flag variables in this snippet?

Question

Below snippet is part of a game application's logic and the Update function is called every frame (about 60 times a second).

void Update()
{
    handleInputEvents();
    if (bShow == true)
    {         
        if (bOnce)
        {
            button.show(true);
            bOnce = false;
        }
    }
    else
    {
        if (bOnce == false)
        {
            button.show(false);
            bOnce = true;
        }
    }
}  

Is there a better way to do what the above snippet is doing?

What the snippet is trying to achieve?
This snippet uses a button.show() function to update a virtual button's state. Everytime a physical button is pressed the state of virtual button is set to true(where true means the virtual button shows on screen and false means it doesn't) , as the physical button press is released, the state of the virtual button needs to be updated to false, meaning the virtual button will not show on the screen.

Problem: If bOnce flag check is not used, then the button.show() will be called every frame leading to more CPU cycles wasted.

The snippet shows my attempt to solve the problem.

Answer 1

Can I lessen the use of boolean flag variables in this snippet?

Consider 1st comparing.

// bShow bOnce  Action
//   0     0    button.show(false); bOnce = true;
//   0     1    nothing
//   1     0    nothing
//   1     1    button.show(true); bOnce = false;
void Update() {
    handleInputEvents();
    if (bShow == bOnce) {         
      button.show(bShow); // or button.show(bOnce) whichever you find more clear.
      bOnce = !bOnce;
    }
}  

Answer 2

Each call to handleInputEvents() can set bShow to a new boolean value.

We could approach the display update task by making a truth table.

| show | once  ||     XOR |     bothTrue | bothFalse | bT || bF |
|------+-------++---------+--------------+-----------+----------|
|    0 |    0  ||       0 |            0 |         1 |        1 |
|    0 |    1  ||       1 |            0 |         0 |        0 |
|    1 |    0  ||       1 |            0 |         0 |        0 |
|    1 |    1  ||       0 |            1 |         0 |        1 |

Looks like you want

    if (!(bShow ^ bOnce))
    {         
        button.show(bShow & bOnce);
        bOnce = !(bShow & bOnce);
    }

But those last two lines are already conditional, so this suffices:

    {         
        button.show(bShow);
        bOnce = !bOnce;
    }

Now the meaning is far more apparent. We're simply copying, then flipping. And what does the XOR mean? It indicates "nothing changed", so its negation is "something changed" which the if body must deal with.

---

The bOnce state variable seems a clumsy way to express Author's Intent. Yes, I understand that you want to call button.show() just once per change. But why not express that notion directly?

Let bDisplayed cache the most recent .show() value we sent to the button. Our goal is to keep the button in sync with bShow.

    handleInputEvents();
    if (bDisplayed != bShow)
    {         
        bDisplayed = bShow;
        button.show(bDisplayed);
    }

Now it is transparently obvious to the Gentle Reader that bDisplayed closely tracks bShow, and what is displayed always matches bDisplayed. Keep the reader in mind when writing code.

Programs must be written for people to read, and only incidentally for machines to execute.

-- SICP, by Abelson and Sussman

Answer 3

I'm not familiar with unity so, I can't review your code from that perspective. But we can certainly make this code more concise. Let me show you how to get started with refactor this code.

1. Get rid of nested branching

Instead of having two levels of branching you can combine the conditions like this:

if (bShow && bOnce)
{         
    button.show(true);
    bOnce = false;
}

2. Transform your else and if blocks to else if

It is basically the same thing what we did in the previous step

else if (!bShow && !bOnce)
{
    button.show(false);
    bOnce = true;
}

3. Take a step back

Now we have only two branches so, it is much easier to overview what we have:

• If both variables are true then show button and set bOnce to false
• If both variables are false then hide button and set bOnce to true
• Otherwise do nothing

4. Combine the conditions

We can combine the two conditions by saying either both true or both false

var bothTrue = bShow && bOnce;
var bothFalse = !bShow && !bOnce;
if (bothTrue || bothFalse)

Now, we just have to figure out how do we need to set button.show and bOnce values:

• The button.show is true if bothTrue
• The bOnce is true if bothFalse

5. Let's put all this together

void Update()
{
    handleInputEvents();

    var bothTrue = bShow && bOnce;
    var bothFalse = !bShow && !bOnce;
    if (bothTrue || bothFalse)
    {         
        button.show(bothTrue);
        bOnce = bothFalse;
    } 
}

I hope it helped you a bit. :)

---

UPDATE #1

As it was mentioned by user16217248 the condition can be simplified. If either both are true or both are false, that means their values are the same (if (bShow == bOnce)). Because booleans could be either true or false.

If both of the variables are declared as bool? or Nullable<bool> then the shortened condition might not be suitable because in that case the condition would be evaluated as true if both are null. Which may or may not be the right solution depending on your requirements.

Answer 4

Why don't you make button.show() simply do an early exit if the visibility state being passed in is already the same? I'm surprised it doesn't already work this way.

void Button::show(bool visible) {
    if (visible == _currentVisibility) {
        return;
    }
    // complicated CPU intensive logic below
    _currentVisibility = visible;
}

Then you can skip all these variable checks and say:

void Update()
{
    handleInputEvents();
    button.show(bShow);
}

Can't change the Button control? Make a wrapper or use inheritance

class ButtonWrapper : public Button {
    std::optional<bool> _visibility;
    void ShowFast(bool visibility) {
        if (_visibility.hasValue() && *_visibility == visibility) {
            return
        }
        show(visibility);
        *_visibility = visibility;
    }
}

Or

class ButtonWrapper {
    std::optional<bool> _visibility;
    Button* _b;
public:
    ButtonWrapper(Button *b) : _b(b) {}
    void ShowFast(bool visibility) {
        if (_visibility.hasValue() && *_visibility == visibility) {
            return
        }
        _b->show(visibility);
        *_visibility = visibility;
    }
}

Answer 5

Since this is tagged unity3d, rather than setting a flag somewhere else in the script, you can just do the following:

void Update()
{
    if (Input.GetButtonDown("<physical button name>")
        button.show(true);

    if (Input.GetButtonUp("<physical button name>")
        button.show(false);
}

Input.GetButtonDown only returns true the first frame the button is pressed; likewise for Input.GetButtonUp.

A more comprehensive solution might be using the Unity Input System, which is more event driven, but might also be overkill if you don't need the flexibility the system provides.

Answer 6

You currently have two booleans, capable of representing 4 states (like many other answers have shown with helpful truth tables). But there's actually only 3 cases here: show, hide, and do nothing. If you can change how handleInputEvents works, one nullable boolean should be enough to handle it all:

bool? bShow = null;

void Update() {
    handleInputEvents();

    if (bShow.HasValue) {
        button.show(bShow.Value);
        bShow = null;
    }
}

void handleInputEvents() {
    // Set bShow to true if there's been input to show the button,
    // set it to false if there's been input to hide the button,
    // and otherwise don't set it at all.
}

Unrelated, but I can't help pointing this out:

Unless you need the bShow value elsewhere in your code, I would recommend making handleInputEvents return the data representing the results (just the bool?, or a struct or a tuple if it resolves multiple things), instead of assigning to a field and reading it later. That way the dependencies are clear – your button-showing code depends on input handling.

// For example
struct InputResult {
    public bool? showButton;
    public bool? sayHello;
}

void Update() {
    var result = handleInputEvents();

    if (result.showButton.HasValue) {
        button.show(result.showButton.Value);
    }
}

InputResult handleInputEvents() {
    var result = new InputResult();
    // Populate the struct
    return result;
}

An even better option could be inverting control and using event-driven/reactive programming, where you can get rid of booleans like this altogether. :)

Answer 7

I'd do:

void Update()
{
    handleInputEvents();
    if (bShow != bShown) {   // if the display doesn't match reality
        button.show(bShow);  // update the display to match
        bShown = bShow;      // and remember that for future checks
    }
}

(personally, I'm not fond of prefixing variable names with their type, but I take it that is a convention for the technology you are working in?)

Answer 8

I think a higher level review of how this whole button thing works.

I want to express a slightly different approach and instead of bShow and bOnce (which are not that clearly defined, especially bOnce) and use physical_button and virtual_button defined as below.

• physical_button the most recent sampled electrical signal of the real physical button.

• virtual_button is the state of the displayed button that the user sees.

I'd like to use 3 states: 0, 1 with their usual meaning of off/on and -1 implying unknown. This -1 useful for initialization.

int physical_button = -1;  // or perhaps type signed char for these 2.
int virtual_button = -1;

void UpdateAlt() {
  handleInputEvents();
  // Unless the physical button has been sampled at least once, nothing to do.
  // ... and display deserves to remain in its initial state. 
  if (physical_button >= 0) { // This test useful if physical_button can be reset to -1.
    if (virtual_button != physical_button) {
      virtual_button = physical_button;
      button.show(virtual_button);
    }
  }
}