I’m trying to implement a web interface for a user database. Hosts can create guests for their courses, the guests can get deleted after the course has ended but have to remain in the database for a given amount of time. If a host tries to create a guest that already exists in the database, the following things can happen:
- The guest is already active and was created by the same host,
- The guest is already active and was created by a different host,
- The guest is in deleted state and was created by the same host,
- The guest is in deleted state and was created by a different host.
Currently, I’m trying to represent the user state in this enum:
public enum GuestUserStatus
{
Unknown = 0,
NewGuest = 1,
ActiveSameHost = 10,
ActiveDifferentHost = 11,
DeletedSameHost = 20,
DeletedDifferentHost = 21
}
This allows me to switch() on the set user state.
However, to set the user state, I’m currently using repetetive if clauses like these:
if (queriedUser.GuestDetails.Creator == creator)
{
if (queriedUser.GuestDetails.State == GuestState.Deleted)
{
currentStatus = GuestUserStatus.DeletedSameHost;
}
else
{
currentStatus = GuestUserStatus.ActiveSameHost;
}
}
else
{
if (queriedUser.GuestDetails.State == GuestState.Deleted)
{
currentStatus = GuestUserStatus.DeletedDifferentHost;
}
else
{
currentStatus = GuestUserStatus.ActiveDifferentHost;
}
}
The nested if clauses act on the same condition but with a different outcome.
Is there a way to avoid this? I looked into marking the enum with [Flags] and using powers of 2 as underlying values, but then I couldn’t use the switch() anymore.
UPDATE: Please note that I have the same conditions twice and different outcomes to them.
UPDATE2: Trying to address the linked question How to tackle a ‘branched’ arrow head anti-pattern?: Let’s say I implement the accepted answer of that question. That would give me something like this:
bool activeSameHost = (queriedUser.Guest.Creator == creator && queriedUser.GuestDetails.State != GuestState.Deleted);
bool activeDifferentHost = (queriedUser.Guest.Creator == creator && queriedUser.GuestDetails.State == GuestState.Deleted);
bool inactiveSameHost = (queriedUser.Guest.Creator != creator && queriedUser.GuestDetails.State == GuestState.Deleted);
bool inativeDifferentHost = (queriedUser.Guest.Creator != creator && queriedUser.GuestDetails.State == GuestState.Deleted);
if(activeSameHost) currentStatus = GuestUserStatus.ActiveSameHost;
if(inactiveSameHost) currentStatus = GuestUserStatus.InactiveSameHost;
if(activeDifferentHost) currentStatus = GuestUserStatus.ActiveDifferentHost;
if(inactiveDifferentHost) currentStatus = GuestUserStatus.InactiveDifferentHost;
I fail to see the improvement. Sure, the ifs are no longer nested, but the only thing that changed is the complexity moving from the nested ifs to the assignment of intermediate variables.
16
You can use [Flags] and switch at the same time, simply by defining an alias for the flag combination that interest you.
[Flags]
public enum GuestUserStatus
{
Unknown = 0,
NewGuest = 1,
NotNew = 2, // I need this to distinguish from the Unknown and NewGuest states.
Deleted = 4,
DifferentHost = 8,
ActiveSameHost = NotNew,
ActiveDifferentHost = NotNew | DifferentHost,
DeletedSameHost = NotNew | Deleted,
DeletedDifferentHost = NotNew | Deleted | DifferentHost,
}
However, in such a design I would try to put the NotNew state at 0 instead of Unknown, but I do not know your requirements. (What is wrong with modeling it with two booleans instead?)
2
Using lamdba actions is another option.
Here is a basic method that checks for conditional and runs appropriate code —
but notice that in state and creator parameters I took a long shot at their type:
they are not necessary good. Just illustrating the concept.
private void SetConditionalValue(Enum state, string guestCreator, string creator, Action activeSameHost, Action inactiveSameHost, Action activeDifferentHost, Action inactiveDifferentHost)
{
if (guestCreator == creator)
{
if (state == GuestState.Deleted)
{
inactiveSameHost.Invoke();
}
else
{
activeSameHost.Invoke();
}
}
else
{
if (state == GuestState.Deleted)
{
inactiveDifferentHost.Invoke();
}
else
{
activeDifferentHost.Invoke();
}
}
}
Now the calls look like this :
SetConditionalValue(queriedUser.GuestDetails.State,
queriedUser.Guest.Creator,
creator,
() => currentStatus = GuestUserStatus.ActiveSameHost,
() => currentStatus = GuestUserStatus.InactiveSameHost,
() => currentStatus = GuestUserStatus.ActiveDifferentHost,
() => currentStatus = GuestUserStatus.InactiveDifferentHost);
You can then reuse the same method for different actions with the same condition:
SetConditionalValue(queriedUser.GuestDetails.State,
queriedUser.Guest.Creator,
creator,
() => subscriptionCost = 130d,
() => subscriptionCost = 215d,
() => subscriptionCost = 125d,
() => subscriptionCost = 100d);
Condition will be re-evaluated but still you don’t have to worry about re writing it every single time. If the condition changes slightly later on, you change in one place and the rest is automatically affected. Of course you can switch to delegates for more control and reference parameters, but if everything is local you don’t need to. You can mix that with pre evaluation of conditions in your update but I think the full if conditional is more readable and sustainable.