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?

asked May 19, 2022 at 12:50
\$\endgroup\$
3
  • \$\begingroup\$ Kind of off topic, but I find that I prefer ModeInUse, so that I don't end up with ! ModeNotInUse. \$\endgroup\$ Commented May 20, 2022 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\$ Commented May 20, 2022 at 7:42
  • \$\begingroup\$ @Vogel612 oh yes, that's a typo, really embarrassed that I missed that \$\endgroup\$ Commented May 20, 2022 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.

answered May 19, 2022 at 17:00
\$\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
};
answered May 19, 2022 at 16:49
\$\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\$ Commented May 22, 2022 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
}
answered May 19, 2022 at 18:33
\$\endgroup\$
13
  • \$\begingroup\$ Too much math, not enough logic \$\endgroup\$ Commented May 20, 2022 at 2:02
  • \$\begingroup\$ @jmoreno What do you mean by not enough logic? \$\endgroup\$ Commented May 20, 2022 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\$ Commented May 20, 2022 at 10:20
  • 1
    \$\begingroup\$ Both of those comments are good, I would suggest putting them in your answer. \$\endgroup\$ Commented May 20, 2022 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\$ Commented May 21, 2022 at 4:51

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.