none
[Flags] to check more bits, how? RRS feed

  • Question

  • Hello,
    [Flags]
    public enum StatePosition
    {
    	ProcessedNot = 0,
    	ProcessedAlready = 1,
    
    	ProcessedAndReadIt = ProcessedAlready | 2,
    	ProcessedAndNotReadIt = ProcessedAlready | 4,
    	ProcessedIsBad = ProcessedAlready | 8
    }
    Step1:
    I fill my position depend from the state in a own list.
    StatePosition currentStatePosition = StatePosition.ProcessedAlready;
    
    if (stateBadmark == "BAD")
    	currentStatePosition = currentStatePosition | StatePosition.ProcessedIsBad;
    
    if (stateOK == "PASS")
    	currentStatePosition = currentStatePosition | StatePosition.ProcessedAndReadIt;
    
    Step2: 
    I go through my list and output the status, depending on which bits are set.
    The goal is to trace the status correctly depending on the bit.
    F   B  X and so one.
    foreach (var item in ListBoards.OrderBy(x => x.SerialId).ThenBy(b => b.Pos).ToList())
    {
    	if (item.State.HasFlag(StatePosition.ProcessedAlready & StatePosition.ProcessedAndNotReadIt))
    		state = "F";
    	if (item.State.HasFlag(StatePosition.ProcessedAlready & StatePosition.ProcessedAndReadIt & StatePosition.ProcessedIsBad))
    		state = "B";
    	
    How do I do that right? I get into each if branch, although some bits are not set. 
    I need a AND check.

    Is there a way to better represent it in debug mode?
    If yes, how?
    Like that or must I cast to int?

    Bit Problem


    Or is that better?
    //if (sb.State.HasFlag(StatePosition.ProcessedAlready))
    if ((sb.State & StatePosition.ProcessedAlready) == StatePosition.ProcessedAlready)
       
    Thanks for help.
    With best regards Markus



    Wednesday, October 10, 2018 4:09 PM

Answers

  • I would go with your updated definition of StatePosition (not StatePosition2) given the scenario you describe. 

    Given your last block statement then it'll set it to P if ProcessedAndReadIt is set. Then later on it might change that to B or X. So the logic seems like it'll work. But personally I find it a little hard to read and you'll be taking the (very minimal hit) of checking the flag 4 times. To me it might just be easier to reorganize your if statements so you only have to check the flag once.

    //Not tested
    if (item.State.HasFlag(StatePosition.ProcessedAlready))
    {
       if (item.State.HasFlag(StatePosition.ProcessedAndReadIt))
          state = item.State.HasFlag(StatePosition.ProcessedIsBad) ? "B" : "P";
       else if (item.StateHasFlag(StatePosition.ProcessedAndNotReadIt))
          state = item.State.HasFlag(StatePosition.ProcessedIsBad) ? "X" : "F";
    };
    It cuts down on the checks a little but I think it is more readable.



    Michael Taylor http://www.michaeltaylorp3.net

    Thursday, October 11, 2018 5:20 PM
    Moderator

All replies

  • HasFlag simply does an AND check for you. So if you want to check multiple conditions you could call it multiple times. But if you wanted to check for both cases you could also OR the flags together and then make one call to HasFlag. You're using & which would only set the bits that both flags shared.

    var isSet = currentStatePosition.HasFlag(StatePosition.ProcessedAlready | StatePosition.ProcessedIsBad);

    But I'll be honest with you, your enum isn't right so I think you need to rethink this implementation.

    1) You're values after ProcessedAlready will auto include that attribute so there is no reason to have to include it in additional HasFlag calls. You can simply use one of your later values and ProcessedAlready has to be set (because it is included in the flag value).

    2) Flags are used to combine different sets of values together. If they cannot be combined together then they shouldn't be in the enum. In your case you are defining ProcessedAndReadIt and ProcessedAndNotReadIt as flags so it is very easy to have an entry that has both.

    var position = StatePosition.ProcessedAndReadIt | StatePosition.ProcessedAndNotReadIt;

    These are mutually exclusive optiosn (given the name) but your position can be set to both. 

    3) You're mixing concerns here. In some cases you can use different ranges of the flag to convey different things (e.g. file access is first byte, file mode is second byte, etc). But they have to be combinable. In your case you're trying to track the "state" of something and whether you've read it or not and whether it was bad or not. They don't really all belong together in my opinion. I'm not really sure your rationale behind trying to use an enum but I think you might be trying to use it when it isn't appropriate. Even the enum name doesn't (to me) match the values in it.

    I would personally consider creating a wrapper type that has your data in it combined with perhaps a state enum that is just for state. The read vs not read seems like it could just be a boolean property on the wrapper type. The bad indicator would certainly be that way as well. Right now your enum would allow you to say the item is bad even though you've never processed it. 

    Part of this also depends on what your code is going to do with it and how you might handle errors. If you are getting data from somewhere (database, API, etc) then if it only returns unprocessed data you don't really need to track that state. You would probably need to track whether it was bad or not though. And what if it is bad, do you want to support trying again later? If so then a toggleable bad indicator would make sense. 

    In general you should use regular properties of the appropriate type for your data. If you find yourself needing some integral values to track information and these values are constant then you should consider moving the "int" values to enums. I wouldn't start with an enum out of the box.


    Michael Taylor http://www.michaeltaylorp3.net

    Wednesday, October 10, 2018 6:16 PM
    Moderator
  • Hi Markus,

    is the following code working like you expect it?

    	if (item.State.HasFlag(StatePosition.ProcessedAlready | StatePosition.ProcessedAndNotReadIt))
    		state = "F";
    	if (item.State.HasFlag(StatePosition.ProcessedAlready | StatePosition.ProcessedAndReadIt | StatePosition.ProcessedIsBad))
    		state = "B";

    If not I may missunderstod the question...

    Greetings, Chris

    Wednesday, October 10, 2018 6:37 PM
  • ProcessedNot = 00000000 = 0

    ProcessedAlready = 00000001 = 1

    ProcessedAndReadIt = 00000011 = 3

    ProcessedAndNotReadIt = 00000101 = 5

    ProcessedIsBad = 00001001 = 9

    Am I correct? (this is little endian)

    Do you want to do it like this: 00000001 | 00000011 = 00000011 // 1 | 3 = 3

    or this: 00000001 & 00000011 = 00000001 ? // 1 & 3 = 1

    Maybe I can help you better if I understand what you are trying to do.

    & Works like this ...11 & ...01 = (1 and 0) = 0 , (1 and 1) = 1, so the result is ...01

    | Works like this ...11 | ...01 = (1 or 0) = 1 , (1 or 1) = 1, so the result is ...11

    You can see 1 as true and 0 as false then it's like a boolean condition.

    Greetings, Chris




    • Edited by DerChris88 Wednesday, October 10, 2018 8:39 PM
    Wednesday, October 10, 2018 6:55 PM
  • Hello Chris, Michael,
    Thank you for your answers, thanks for your help
    The goal is when the position has set the property.
    ProcessedAlready   &
    ProcessedAndReadIt &
    ProcessedIsBad
       --> to trace a "B"
    
    ProcessedAlready   &
    ProcessedAndReadIt
    
    to trace a "P"
    
    The error was at the & instead |
     | is right.
    
      Index |      State |  
          1 |          F |  
          1 |          F |  
          2 |          F |  
          3 |          B |  
          4 |          X |  
    	  5 |          P |  
    	  6 |          P |  
     
     Works now as expected.
     The position can occur multiple times, if one is good the next with the matching index is bad, 
     so both are bad.
     
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     
     CoolDadTx means I do not need that. ProcessedAlready
    
     Is that the position may not have been processed yet. Then I see that if the flag is not set.
     OK or if the value != 0 then is processed.
     
     How is your opinion?
     
     
    [Flags]
    public enum StatePosition
    {
    	ProcessedNot = 0,
    	ProcessedAlready = 1,
    
    	ProcessedAndReadIt = ProcessedAlready | 2,
    	ProcessedAndNotReadIt = ProcessedAlready | 4,
    	ProcessedIsBad = ProcessedAlready | 8
    }
    
    0000   0
    0001   1    ProcessedAlready
    0011   3    ProcessedAndReadIt
    0101   5    ProcessedAndNotReadIt
    1001   9    ProcessedIsBad
    
     if (item.State.HasFlag(SingleBoard.StatePosition.ProcessedAlready | 
          StatePosition.ProcessedAndReadIt | 
    	  StatePosition.ProcessedIsBad))
    	  
                        state = "B";
    					
    					0001
    					0011
    					0101
    					----------   OR
    					0111
    					
    					0001
    					0011
    					0101
    					----------   AND
    					0001   ProcessedAlready is 1, so all has this, so all has the state P
    
    [Flags]
    public enum StatePosition2
    {
    	ProcessedNot = 0,
    	ProcessedAndReadIt = 2,
    	ProcessedAndNotReadIt = 4,
    	ProcessedIsBad = 8
    }
    
    0000   0
    0010   2
    0100   4
    1000   8
    
    // ------------------------------
    
    But, can I see in the debugging mode the value like that? ProcessedAndNotReadIt | ProcessedIsBad
    
    1100 -> 12 or only with 12 as int?
    
    
    // ------
    My final solution.
    foreach (var item in ListBoards.OrderBy(x => x.SerialIdIndex).ThenBy(b => b.Pos).ToList())
    {
    	//P = Pass
    	//F = Fail
    	//B = Pass and Bad
    	//X = Fail and Bad
    
    	state = "-";
    
    	if (item.State.HasFlag(StatePosition.ProcessedAlready))
    	{
    		if (item.State.HasFlag(StatePosition.ProcessedAndReadIt))
    			state = "P";
    		if (item.State.HasFlag(StatePosition.ProcessedAndNotReadIt))
    			state = "F";
    		if (item.State.HasFlag(StatePosition.ProcessedAndReadIt | StatePosition.ProcessedIsBad))
    			state = "B";
    		if (item.State.HasFlag(StatePosition.ProcessedAndNotReadIt | StatePosition.ProcessedIsBad))
    			state = "X";
    	}
    Can I improve it?
    With best regards Markus



    Thursday, October 11, 2018 5:02 PM
  • I would go with your updated definition of StatePosition (not StatePosition2) given the scenario you describe. 

    Given your last block statement then it'll set it to P if ProcessedAndReadIt is set. Then later on it might change that to B or X. So the logic seems like it'll work. But personally I find it a little hard to read and you'll be taking the (very minimal hit) of checking the flag 4 times. To me it might just be easier to reorganize your if statements so you only have to check the flag once.

    //Not tested
    if (item.State.HasFlag(StatePosition.ProcessedAlready))
    {
       if (item.State.HasFlag(StatePosition.ProcessedAndReadIt))
          state = item.State.HasFlag(StatePosition.ProcessedIsBad) ? "B" : "P";
       else if (item.StateHasFlag(StatePosition.ProcessedAndNotReadIt))
          state = item.State.HasFlag(StatePosition.ProcessedIsBad) ? "X" : "F";
    };
    It cuts down on the checks a little but I think it is more readable.



    Michael Taylor http://www.michaeltaylorp3.net

    Thursday, October 11, 2018 5:20 PM
    Moderator