1

I wanted to toggle a control on a C# Winform, and wrote it this way:

private void chtCPU_Click(object sender, EventArgs e)
    {
        bool is3d = chtCPU.ChartAreas["ChartArea1"].Area3DStyle.Enable3D;

        chtCPU.ChartAreas["ChartArea1"].Area3DStyle.Enable3D = !is3d;
    }

Would this be considered a good method for accomplishing this? It seems highly readable, and efficient, but is it good approach?

Austin T French
  • 5,022
  • 1
  • 22
  • 40
  • possible duplicate of [Easiest way to flip a boolean value?](http://stackoverflow.com/questions/610916/easiest-way-to-flip-a-boolean-value) – Federico Berasategui Aug 29 '13 at 17:53
  • 2
    Yes, value = !value; is indeed correct to toggle a bool. But please post "Is X a good practice?" on [Code Review](http://codereview.stackexchange.com/) and keep SO for specific programming problems. – Pierre-Luc Pineault Aug 29 '13 at 17:55
  • 1
    This question appears to be off-topic because it is about reviewing working code. – Servy Aug 29 '13 at 17:57

2 Answers2

5

It's OK. I would probably assign part of the expression to a variable to avoid repeating myself.

var style = chtCPU.ChartAreas["ChartArea1"].Area3DStyle;
style.Enable3D = !style.Enable3D;
driis
  • 161,458
  • 45
  • 265
  • 341
4

You can either use driis's method, which I highly recommend as the best tradeoff between being readable and concise, or if you really want a one-liner...

You can use XOR with true to accomplish this succinctly.

chtCPU.ChartAreas["ChartArea1"].Area3DStyle.Enable3D ^= true; //toggle
Timothy Shields
  • 75,459
  • 18
  • 120
  • 173
  • Short and elegant, yes, I would argue it is not the most easily read way to do it. – driis Aug 29 '13 at 17:56
  • Not necessarily immediately recognizable for what it does, though. I would argue that even though this is short, I personally wouldn't prefer it. – Steve Aug 29 '13 at 17:58
  • The fact that you need the comment means that this should not be used. Creative, though. – usr Aug 29 '13 at 19:40
  • Oh and by the way this does not work with arbitrary booleans. Bools can be different from 0 and 1. If it was (bool)2, you would not toggle it this way. – usr Aug 29 '13 at 19:43
  • @usr A C# `bool` has two possible values: `true` and `false`. I don't know what you're talking about... – Timothy Shields Aug 29 '13 at 19:43
  • @TimothyShields at the CLR level it does not :) It is a byte at the IL level. You can construct a repro using reflection emit or ilasm. There are 255 values which evaluate to true. (I knew you would say that. Everyone does when I remark about this). – usr Aug 29 '13 at 19:57
  • @usr You're wrong about this. Read here: http://msdn.microsoft.com/en-us/library/zkacc7k1.aspx The two types being operated on are both `bool`, and the `^` operator has a special overload for two `bool` operands. In other words, if you use `^` with numeric types, it will do a bitwise XOR, but if you do it with `bool`s, it will do a *logical* XOR. – Timothy Shields Aug 29 '13 at 20:02
  • Who knew! When Microsoft Pex came out this issue was made visible because Pex can generate complex bool values.; Update: Not according to ildasm: `bool b = args.Length != 0; return (b ^ b) ? 1 : 2;` Look at the IL. A normal `xor`. I'm not going to invest the time to prove it but the C# docs are probably incorrect. – usr Aug 29 '13 at 20:12
  • Ok here is a repro http://pastebin.com/dhiFqxTs Prints `1`. IOW true ^ true is equal to true. – usr Aug 29 '13 at 20:16
  • @usr No duh you can break it with `unsafe` code. I hadn't considered that was even up for discussion. You can break any code if you enable `unsafe`. – Timothy Shields Aug 29 '13 at 20:18
  • What I'm saying is, a CLR bool is a byte and that is perfectly safe and verifiable. How the bool was created does not matter. You can use ilasm to create a verifying, low-trust (bool)2. My point holds. My repro just uses unsafe to save labor. – usr Aug 29 '13 at 20:19
  • @usr And I can do `List L = new List(); *((int*)&L) = 42; L.Add(7);`. Look, the .NET `List` type is garbage. That's analogous to what you're saying. According to specification, a `bool` can have only two possible values: `true` and `false`. If by using `unsafe` code you hack the area of memory where a `bool` is to contain bits that do not match the bits of `true` or `false` (whatever those may be in the underlying implementation), then you've gone against the specification of what a `bool` is and you are now enjoying **undefined behavior**. – Timothy Shields Aug 29 '13 at 20:26
  • Why is it possible to create a (bool)2 in IL and have it validate in low-trust, then? Nothing unsafe about it. My repro just takes a shortcut. – usr Aug 29 '13 at 20:29