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?
3 Answers 3
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.
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
};
-
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– radarbob2022年05月22日 03:33:31 +00:00Commented May 22, 2022 at 3:33
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
}
-
\$\begingroup\$ Too much math, not enough logic \$\endgroup\$jmoreno– jmoreno2022年05月20日 02:02:23 +00:00Commented May 20, 2022 at 2:02
-
\$\begingroup\$ @jmoreno What do you mean by not enough logic? \$\endgroup\$Peter Csala– Peter Csala2022年05月20日 04:20:11 +00:00Commented 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\$iSR5– iSR52022年05月20日 10:20:22 +00:00Commented May 20, 2022 at 10:20 -
1\$\begingroup\$ Both of those comments are good, I would suggest putting them in your answer. \$\endgroup\$jmoreno– jmoreno2022年05月20日 11:51:53 +00:00Commented 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\$radarbob– radarbob2022年05月21日 04:51:46 +00:00Commented May 21, 2022 at 4:51
ModeInUse
, so that I don't end up with! ModeNotInUse
. \$\endgroup\$