2
\$\begingroup\$

I have a class with a "mode" state. There are currently two modes and we are not likely to add more in the future.

The class has a CurrentMode property, and a function to swap to the other mode. I have implemented this using another property called ModeNotInUse which returns the opposite value of CurrentMode.

However, I am in two minds about how to implement this ModeNotInUse

public enum PlayerMode
{
    Puzzle,
    Action
}

public class ModeController
{
    public PlayerMode CurrentMode { get; private set; }

   
    private PlayerMode ModeNotInUse => ...


    public void SwitchMode()
    {
        CurrentMode = ModeNotInUse
    }

}

My two implimentations of this are:

private PlayerMode ModeNotInUse 
   => CurrentMode switch
   {
       PlayerMode.Puzzle => PlayerMode.Action,
       _ => PlayerMode.Puzzle
   };
   

Or

private PlayerMode ModeNotInUse 
    => CurrentMode == PlayerMode.Puzzle 
        ? PlayerMode.Action 
        : PlayerMode.Puzzle;

Which better captures my intent here? The first is my preference but I worry it's difficult to read.


(edit) I could of course do this with a backing boolean field but that feels even less clean

private bool inPuzzleMode;
public PlayerMode CurrentMode
{
    get => inPuzzleMode switch
        {
            true => PlayerMode.Puzzle,
            false => PlayerMode.Action
        };
    private set => inPuzzleMode = value == PlayerMode.Puzzle;
}

private PlayerMode ModeNotInUse 
   =>  inPuzzleMode switch
   {
       true => PlayerMode.Action,
       false => PlayerMode.Puzzle
   };

Which do you think is best?

\$\endgroup\$
3
  • \$\begingroup\$ Kind of off topic, but I find that I prefer ModeInUse, so that I don't end up with ! ModeNotInUse. \$\endgroup\$
    – Teepeemm
    May 20 at 0:54
  • \$\begingroup\$ It's suspicious to me that you'd need the mode that is currently not in use outside of the ModeController... \$\endgroup\$
    – Vogel612
    May 20 at 7:42
  • \$\begingroup\$ @Vogel612 oh yes, that's a typo, really embarrassed that I missed that \$\endgroup\$
    – Jay
    May 20 at 10:04

3 Answers 3

5
\$\begingroup\$

I think a method would be better than a property in this case. However, I would do it this way :

public class ModeController
{
    public PlayerMode CurrentMode { get; private set; }

    public void SwitchMode() 
          => CurrentMode == PlayerMode.Puzzle ? PlayerMode.Action : PlayerMode.Puzzle;
}

if the PlayerMode is used in different classes, and you want to get the opposite mode, then you can move SwitchMode() into an extension :

public static class Extension {
    public static PlayerMode  SwitchMode(this PlayerMode current) 
            => current == PlayerMode.Puzzle ? PlayerMode.Action : PlayerMode.Puzzle;    
}

public class ModeController
{
    public PlayerMode CurrentMode { get; private set; }

    public void SwitchMode() => CurrentMode = CurrentMode.SwitchMode();
}

if there is a requirement where you want to know the old mode, then you could store the old mode before switching it, and add another method to get the old mode.

\$\endgroup\$
4
\$\begingroup\$

First, kudos to you for using an enum even though it would be very tempting to just use a bool in its place. Not enough can be said for semantic programming.

Second, I would use your first method, but perhaps extract it to an extension method:

public static PlayerMode NotInUse(this PlayerMode currentMode) => currentMode switch
{
    PlayerMode.Puzzle => PlayerMode.Action,
    _ => PlayerMode.Puzzle
};
\$\endgroup\$
1
  • 1
    \$\begingroup\$ User system terminology - best way to say what the code means. However a more-or-less leaky abstraction is inevitable. Whatever he vote counts, the "best" answer will start from "semantic programming" and end with "comments and (automated) tests." \$\endgroup\$
    – radarbob
    May 22 at 3:33
3
\$\begingroup\$

How about simply just this?

=> (PlayerMode)((CurrentMode + 1) % 2);

Here you are taking advantage of the underlying values: 0 and 1.


UPDATE #1

I think if you look at this problem for the domain perspective then the whole idea of ModeNotInUse is pointless. Since if you want to branch based on the mode then you

  • either use CurrentMode == PlayerMode.Puzzle
  • or CurrentMode != PlayerMode.Puzzle

and you have covered all cases without introducing a redundant property.

My solution is more fragile than others because if you set explicitly the value for the PlayerMode enums (different than the default ones) then it will not work anymore. For example:

public enum PlayerMode
{
    Puzzle = 1,
    Action = 10
}
\$\endgroup\$
13
  • \$\begingroup\$ Too much math, not enough logic \$\endgroup\$
    – jmoreno
    May 20 at 2:02
  • \$\begingroup\$ @jmoreno What do you mean by not enough logic? \$\endgroup\$ May 20 at 4:20
  • \$\begingroup\$ @jmoreno actually I think it's a clever idea, and it's feasible approach since the OP has stated that PlayerMode will never have any future changes. \$\endgroup\$
    – iSR5
    May 20 at 10:20
  • 1
    \$\begingroup\$ Both of those comments are good, I would suggest putting them in your answer. \$\endgroup\$
    – jmoreno
    May 20 at 11:51
  • 1
    \$\begingroup\$ if you set explicitly the value for the PlayerMode enums (different than the default ones) then it will not work anymore => Explicitly setting to 0 and 1 respectively helps to loudly make, or at least strongly imply, this point IMO. \$\endgroup\$
    – radarbob
    May 21 at 4:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.