3

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
  • 2
    No, it tackles a different question. I have the same condition but different outcomes, the question linked has the same outcomes on different conditions.
    – Thaoden
    Mar 10, 2015 at 8:37
  • 1
    Why do you think that switch and [Flags] cannot be used together? You can write case GuestUserStatus.Deleted | GuesUserStatus.SameHost:, and you can also define DeletedSameHost = Deleted | SameHost in your enum declaration.
    – jhominal
    Mar 10, 2015 at 8:54
  • I am thinking bitmask ...
    – Mawg
    Mar 10, 2015 at 8:56
  • @jhominal I have to pay attention to the order inside the switch if using the bitwise OR: case GuestUserStatus.Deleted will be evaluated and call an action, a later case GuestUserStatus.Deleted | GuestUserStatus.SameHost will be called too. Won't it?
    – Thaoden
    Mar 10, 2015 at 8:58
  • 1
    possible duplicate of How to tackle a 'branched' arrow head anti-pattern?
    – user53019
    Mar 10, 2015 at 19:42

2 Answers 2

11

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
  • This is exactly what I was looking for. Works like a charm. Many thanks!
    – Thaoden
    Mar 11, 2015 at 10:14
  • 2
    Unkown = 0 is a good practise. If you instantiate enumeration, by default, it will always has value of 0. If you match value zero with some valid state, you might end up reading the wrong flag.
    – StupidOne
    Mar 11, 2015 at 14:12
1

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.

Your Answer

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

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