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.

Community
  • 1
Thaoden
  • 151
  • 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 '15 at 08: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 '15 at 08:54
  • I am thinking bitmask ... – Mawg says reinstate Monica Mar 10 '15 at 08: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 '15 at 08:58
  • On a side note, would this question be more fit for StackOverflow? – Thaoden Mar 10 '15 at 09:01
  • @Thaoden: No it will not. case GuestUserStatus.Deleted: will only be executed if the value is exactly equal to Deleted, and case GuestUserStatus.Deleted | GuestUserStatus.SameHost: will only be called if the value is exactly equal to Deleted | SameHost, because in effect it is case (GuestUserStatus.Deleted | GuestUserStatus.SameHost):. – jhominal Mar 10 '15 at 09:01
  • @jhominal Too bad you deleted your original answer, as it helped me. That way, my switch remained the way it was and the nested ifs vanished into nothingness. Thanks! – Thaoden Mar 10 '15 at 09:12
  • Undeleted. Thing was, I also wanted to understand why you thought Flags did not work, because I thought I was missing something. – jhominal Mar 10 '15 at 09:15
  • i would personally use a method that receive the enum and 4 actions that enumerate each condition results. So when you need to compare and run something depending on the enum you use that method and pass the 4 actions it can perform. – Franck Mar 10 '15 at 12:09
  • @Franck I don't quite get what you mean - do you have an example? – Thaoden Mar 10 '15 at 14:21
  • 1
    possible duplicate of How to tackle a 'branched' arrow head anti-pattern? –  Mar 10 '15 at 19:42
  • @GlenH7 See the update on my question. – Thaoden Mar 11 '15 at 09:34
  • I think you should refactor it and not have status and host together. – Pieter B Mar 11 '15 at 10:10
  • @PieterB And then instead of switching on the enum, I have to if-test both on the to be implemented host class and the to be implemented state class. Again, it's only moving complexity around. As per my requirements (see question), I need both informations inside my queriedUser. – Thaoden Mar 11 '15 at 10:22
  • @Thaoden i posted an answer of what i meant. actually use it alot in 3D when i play with the camera and i need some things to change when certain orientation are met – Franck Mar 11 '15 at 12:11
  • Check out this chapter on state objects. – TehShrike Mar 17 '15 at 13:20

2 Answers2

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?)

jhominal
  • 1,468
  • This is exactly what I was looking for. Works like a charm. Many thanks! – Thaoden Mar 11 '15 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 '15 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.

logc
  • 2,190
Franck
  • 111