Shopping cart logic for authenticated users and guests I am tasked with cleaning up someone else's Objective-C code, and will admit it is not my language of choice. I am not sure if this is a valid question but was hoping someone could double check my attempt to clean up a method written by someone else, and make sure I didn't miss some minor detail or over simplify the code.

The original method written by someone else


if([[UserManager sharedManager]isUserLoggedIn] &amp;&amp; ([APPDELEGATE isCommerceZone]))// Auth and commerce
    {
        [self showPopUpAddToCartOrExpressOrder:NO];

        UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification, _addToCartOrExpressOrderView);

    }
    else if([[UserManager sharedManager]isUserLoggedIn] &amp;&amp; (![APPDELEGATE isCommerceZone])) // Auth and noncommerce
    {
          [self onModifyItemDone:nil];

    }
    else if (![[UserManager sharedManager]isUserLoggedIn] &amp;&amp; ([APPDELEGATE isCommerceZone]))// guest and commerce
    {
        [self showPopUpAddToCartOrExpressOrder:YES];
       UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification, _addToCartOrExpressOrderView);

    }
    else if (![[UserManager sharedManager]isUserLoggedIn] &amp;&amp; (![APPDELEGATE isCommerceZone]))// guest and noncommerce
    {
        /// Do nothing

    }



My revision

if([APPDELEGATE isCommerceZone])
    {

        [self showPopUpAddToCartOrExpressOrder:![[UserManager sharedManager]isUserLoggedIn]];

        UIAccessibilityPostNotification(UIAccessibilityScreenChangedNotification,_addToCartOrExpressOrderView);


    } else if([[UserManager sharedManager]isUserLoggedIn]) // Auth and noncommerce
    {
        [self onModifyItemDone:nil];
    }


In Objective-C, is this the equivalent to saying this boolean equals the opposite of this other boolean?

[self showPopUpAddToCartOrExpressOrder:![[UserManager sharedManager]isUserLoggedIn]];
There are two primary concerns I have.



First, rather than accessing [[UserManager sharedManager]isUserLoggedIn] twice, let's access it just once:

BOOL loggedIn = [[UserManager sharedManager] isUserLoggedIn];


Now just use the loggedIn variable in place of calling these methods.  

In this case, it's not going to make a huge difference, but it's a good habit to get into.  If you're going to use the result of a method call multiple times within a method, call the original method once and save the result to a local variable.  This can make the code more clear (with appropriately named local variables) and will be more efficient with more complex code.



Second, APPDELEGATE, I'm going to assume, is a #define-ed macro.  You should eliminate uses of this macro.  For one, you should just generally be avoiding #define-ing stuff, and for two, this runs into the same trouble as calling that isUserLoggedIn method repeatedly, but masks it behind APPDELEGATE.

Given that you're cleaning up someone else's code, I recommend just eliminating uses of the macro, but don't actually delete the macro definition until you've cleaned them all up eventually.

Instead, use a function that returns the app delegate reference.  Call that function once and store a local reference to it for future uses within the method.

A function would look very similar to the macro:

MyAppDelegate * MyAppDelegate() {
    return [[UIApplication sharedApplication] delegate];
}


Now...

MyAppDelegate *appDelegate = MyAppDelegate();


And use appDelegate as your reference to app delegate.