views:

3142

answers:

10

I just want to flip a boolean based on what it already is. If it's true - make it false. If it's false - make it true.

Here is my code excerpt:

switch(wParam) {

case VK_F11:
  if (flipVal == true) {
     flipVal = false;
  } else {
    flipVal = true;
  }
break;

case VK_F12:
  if (otherVal == true) {
     otherValVal = false;
  } else {
    otherVal = true;
  }
break;

default:
break;
}
+76  A: 

You can flip a value like so:

myVal = !myVal;

so your code would shorten down to:

switch(wParam) {
    case VK_F11:
    flipVal = !flipVal;
    break;

    case VK_F12:
    otherVal = !otherVal;
    break;

    default:
    break;
}
John T
Not only this is the easiest, but also the cleanest way.
sharptooth
I use this Boolean "toggle" all the time in code.
Jim C
succint and clear is the only way code
Jason
the two cases can be merged as they do the same thing.
David Allan Finch
It's always the easiest questions that get such intense rep :P
zildjohn01
Too bad more than half of it is wasted though.. :/
ryeguy
Is default: break; really necessary? Won't the switch end the same without it?
Chris Lutz
Default: break; is unnecessary.
Rob K
If you're toggling something long-winded like object1->system1.system2.system3.parameter1 then it can be helpful to have a TOGGLE(a) macro. This prevents some mistakes and makes it all more readable on narrow screens.
OJW
+7  A: 

Also

if (otherVal == true)

is equivalent to

if(otherVal)

As John T showed, you don't need it here. Just figured I'd point it out.

drby
+2  A: 

I prefer John T's solution, but if you want to go all code-golfy, your statement logically reduces to this:

//if key is down, toggle the boolean, else leave it alone.
flipVal = ((wParam==VK_F11) && !flipVal) || (!(wParam==VK_F11) && flipVal);
if(wParam==VK_F11) Break;

//if key is down, toggle the boolean, else leave it alone.
otherVal = ((wParam==VK_F12) && !otherVal) || (!(wParam==VK_F12) && otherVal);
if(wParam==VK_F12) Break;
JosephStyons
Don't you have to check wParam against VK_F11 and VK_F12?
drby
Doh! Yes, thank you... I've made that edit.
JosephStyons
+3  A: 

The codegolf'ish solution would be more like:

flipVal = (wParam == VK_F11) ? !flipVal : flipVal;
otherVal = (wParam == VK_F12) ? !otherVal : otherVal;
korona
Well if we're going to codegolf: flipVal= (wParam==VK_F11)!=flipVal;...
bobince
ok, you guys win... :) I was never much of a golfer anyway.
JosephStyons
+2  A: 

If you know the values are 0 or 1, you could do flipval ^= 1.

Mike Dunlavey
Why use a bitwise operator for a logical operation? Smells of needless obfuscation to me.
Mark Pim
@Mark: Sorry. Guess I'm old-fashioned. But it does help if your L-value expression is really long, so you don't have to repeat it. Also, you could say flipval ^= TRUE. Is that better?
Mike Dunlavey
@Mark - see my posting - because sometimes your logical values are stored in bits. Not everyone wants to waste a whole 8 bits (or more) just for a boolean.
Alnitak
@Alnitak: You're right in some circumstances. I have seen some people pack bits together to "save space" and act as if the instructions to access them didn't take any space.
Mike Dunlavey
+10  A: 

Clearly you need a factory pattern!

KeyFactory keyFactory = new KeyFactory();
KeyObj keyObj = keyFactory.getKeyObj(wParam);
keyObj.doStuff();


class VK_F11 extends KeyObj {
   boolean val;
   public void doStuff() {
      val = !val;
   }
}

class VK_F12 extends KeyObj {
   boolean val;
   public void doStuff() {
      val = !val;
   }
}

class KeyFactory {
   public KeyObj getKeyObj(int param) {
      switch(param) {
         case VK_F11:
            return new VK_F11();
         case VK_F12:
            return new VK_F12();
      }
      throw new KeyNotFoundException("Key " + param + " was not found!");
   }
}

:D

</sarcasm>
Drew
Hahahahaha. ROFL.
Austin
We could probably add in the singleton pattern for the factory too.
Drew
Oh God why am I laughing so hard?
Orm
@Orm Cause you are an _ORM_? :)
mlvljr
+1 for a creative (and hilarious) anti-solution.
Jough Dempsey
+2  A: 

Just for information - if instead of an integer your required field is a single bit within a larger type, use the 'xor' operator instead:

int flags;

int flag_a = 0x01;
int flag_b = 0x02;
int flag_c = 0x04;

/* I want to flip 'flag_b' without touching 'flag_a' or 'flag_c' */
flags ^= flag_b;

/* I want to set 'flag_b' */
flags |= flag_b;

/* I want to clear (or 'reset') 'flag_b' */
flags &= ~flag_b;

/* I want to test 'flag_b' */
int b_is_set = (flags & flab_b) != 0;
Alnitak
+1  A: 

Clearly you need a flexible solution that can support types masquerading as boolean. The following allows for that:

template<typename T>    bool Flip(const T& t);

You can then specialize this for different types that might pretend to be boolean. For example:

template<>  bool Flip<bool>(const bool& b)  { return !b; }
template<>  bool Flip<int>(const int& i)    { return !(i == 0); }

An example of using this construct:

if(Flip(false))  { printf("flipped false\n"); }
if(!Flip(true))  { printf("flipped true\n"); }

if(Flip(0))  { printf("flipped 0\n"); }
if(!Flip(1)) { printf("flipped 1\n"); }

No, I'm not serious.

dominic hamon
A: 

In similar cases where a clean and elegant solution which is appliable here is not possible you should at least wrap the code into a function and call it instead of copy-pasting. It's ironic, but at least the original version of the question had otherVal identifier mistyped. Copy-pasting in such cases is poor design and asking for trouble.

sharptooth
+2  A: 

This seems to be a free-for-all ... Heh. Here's another varation, which I guess is more in the category "clever" than something I'd recommend for production code:

flipVal ^= (wParam == VK_F11);
otherVal ^= (wParam == VK_F12);

I guess it's advantages are:

  • Very terse
  • Does not require branching

And a just as obvious disadvantage is

  • Very terse

This is close to @korona's solution using ?: but taken one (small) step further.

unwind