Checkout Process for Xamarin E commerce application

Question

I'm new to development (I'm self Taught I got a job recently as an"app Developer") and this is my first real big project. In essence, it's an e-commerce store that uses a very awkward API. My Checkout process is taking 6 seconds per item and you can imagine if I let a user checkout 50+ items, they will think something's wrong.

I'm going to share my whole checkout process along with a question. What are my bottlenecks? Too many loops to make checks? How do I improve something like this?

A few things to note, I have a variable and a single Product. Sometimes the API returns the Stock Quantity of null, which just means that it's unlimited.

        private async void Checkout_clicked(object sender, EventArgs e)
        {
            try
            {
                await Navigation.PushPopupAsync(new LoadingPopup());
            }
            catch (Exception ex)
            {
                Crashes.TrackError(ex);
            }

            try
            {

                if (FullCart.CartList == null || FullCart.CartList.Count <= 0 || !FullCart.CartList.Any())
                {
                    var yx = await DisplayAlert("Whoops",
                                "Cart seem's to be empty, We cant checkout nothing", "Back to Cart", "Supplier");
                    if (yx)
                    {
                    }
                    else
                    {
                        var masterDetailPage = new Home();
                        masterDetailPage.Detail = new NavigationPage(new Suppliers());
                        Application.Current.MainPage = masterDetailPage;
                    }
                }
                else
                {
                    await BeginCheckout();
                }
            }
            catch (Exception ex)
            {
                Crashes.TrackError(ex);
            }

            try
            {
                await Navigation.PopPopupAsync();
            }
            catch (Exception ex)
            {
                Crashes.TrackError(ex);
            }

        }

 private async Task BeginCheckout()
        {
            try
            {
                //You cant checkout if your not logged in There are no Guest Checkouts(I can But would rather not)
                if (Users.LoggedIn && _spamClick == false)
                {
                    if (_orderlineitems == null) _orderlineitems = new List<OrderLineItem>();

                    RestAPI rest = new RestAPI("http://xxxxx/wp-json/wc/v2/",
                    "xxxxxxxxxxxxxxxxxxxxxxxxx", "cs_xxxxxxxxxxxxxxxxxxxxxxxx");

                    var wc = new WCObject(rest);

                    await IsInStock();
                    var order = new Order { status = "on-hold", customer_id = Users.CId };
                    foreach (var item in _simpleCartlist)
                        if (_simpleCartlist.Any(i => i.InStock == false))
                        {
                            try
                            {
                                await Navigation.PopPopupAsync();

                            }
                            catch (Exception ex)
                            {
                                Crashes.TrackError(ex);
                            }

                        }
                        else if (_simpleCartlist.All(i => i.InStock))
                        {
                            if (item.StockStatus == "instock" || item.StockStatus is string stringValue && string.IsNullOrWhiteSpace(stringValue) || item.StockStatus == null)
                            {
                                var a = Convert.ToInt32(item.ProductQuantity);
                                if (item.VariationId <= 0) item.VariationId = item.PId;
                                if (item.StockQuantity == 0)
                                {
                                    _productBoughtOut = true;
                                    _productnames.Add(item.ProductName);
                                }

                                order.line_items = order.line_items ?? new List<OrderLineItem>();
                                order.line_items.Add(new OrderLineItem
                                { product_id = item.PId, variation_id = item.VariationId, quantity = a });
                            }
                            else
                            {
                                try
                                {
                                    await Navigation.PopPopupAsync();

                                }
                                catch (Exception ex)
                                {
                                    Crashes.TrackError(ex);
                                }

                                var yx = await DisplayAlert("Order Cant be Placed",
                                    $"{item.ProductName} is out of stock",
                                    "Back to Cart", "Home");
                                if (yx)
                                {
                                }
                                else
                                {
                                    var masterDetailPage = new Home();
                                    masterDetailPage.Detail = new NavigationPage(new Home());
                                    Application.Current.MainPage = masterDetailPage;
                                }
                            }
                        }

                    if (_productBoughtOut)
                    {
                        try
                        {
                            await Navigation.PopPopupAsync();

                        }
                        catch (Exception ex)
                        {
                            Crashes.TrackError(ex);
                        }

                        var yx = await DisplayAlert("Order Cant be Placed", $"Not enough stock for {_productnames}",
                            "Back to Cart", "Home");
                        if (yx)
                        {
                        }
                        else
                        {
                            var masterDetailPage = new Home();
                            masterDetailPage.Detail = new NavigationPage(new Home());
                            Application.Current.MainPage = masterDetailPage;
                        }
                    }
                    else if (_simpleCartlist.All(i => i.InStock))
                    {
                        if (FullCart.CartList != null && _spamClick == false || _items.Items != null && _spamClick == false)
                        {
                            _spamClick = true;
                            await wc.Order.Add(order);

                            Preferences.Clear("Cart");
                            Preferences.Remove("Cart");
                            FullCart.CartList.Clear();
                            _items.Items.Clear();
                            var masterDetailPage = new Home();
                            masterDetailPage.Detail = new NavigationPage(new Checkedout());
                            Application.Current.MainPage = masterDetailPage;
                        }

                        _spamClick = true;
                    }
                }
                else
                {
                    if (_spamClick)
                    {
                        await DisplayAlert("Woops", "Your trying to order twice", "Ok");
                    }
                    else
                    {
                        var y = await DisplayAlert("Woops", "Please Login to check Out", "Login", "Home");
                        if (y)
                        {
                            var masterDetailPage = new Home();
                            masterDetailPage.Detail = new NavigationPage(new Login());
                            Application.Current.MainPage = masterDetailPage;
                        }
                        else
                        {
                            await Navigation.PushAsync(new Home());
                        }
                    }
                }

            }
            catch (Exception ex)
            {
                Crashes.TrackError(ex);
            }
        }

private async Task SingleCheck()
        {
            try
            {
                foreach (var CartItem in _simpleCartlist)
                    if (_currentListItem == CartItem.PId)
                    {
                        if (_singleProduct.stock_quantity == null)
                        {
                            //CartItem.InStock = true;
                            _singleProduct.stock_quantity = 999999;
                        }
                        if (CartItem.ProductQuantity == 0)
                        {
                            CartItem.ErrorMsg = "Item requires a Quantity of atleast 1" + System.Environment.NewLine + "Quantity Cannot be 0!";
                            await DisplayAlert("Oops", "Whe can't checkout nothing See if a products quantity isnt 0", "Ok");
                            return;
                        }
                        if (_singleProduct.stock_quantity == 0 || _singleProduct.stock_status == "outofstock" ||
                            CartItem.ProductQuantity > _singleProduct.stock_quantity)
                        {
                            try
                            {
                                await Navigation.PopPopupAsync();

                            }
                            catch (Exception ex)
                            {
                                Crashes.TrackError(ex);
                            }

                            CartItem.InStock = false;
                            var AlertResult = await DisplayAlert("Not Enough Products",
                                $"There is this much Stock left: {_singleProduct.stock_quantity} for {_singleProduct.name}",
                                "Back to Cart", "Keep Shopping");
                            CartItem.ErrorMsg = "Product shortage " + $" There is this much Stock left: {_singleProduct.stock_quantity} for {_singleProduct.name}";
                            if (AlertResult)
                            {
                                CartItem.InStock = false;
                                CartItem.StockQuantity = Convert.ToInt32(_singleProduct.stock_quantity);
                                _items = new ItemList(FullCart.CartList);
                                cartView.BeginRefresh();
                                CartItem.StockStatus = _singleProduct.stock_status;
                                cartView.EndRefresh();
                                return;
                            }

                            var masterDetailPage = new Home();
                            masterDetailPage.Detail = new NavigationPage(new Home());
                            Application.Current.MainPage = masterDetailPage;
                        }
                        else if (_singleProduct.stock_quantity != 0 && _singleProduct.stock_status == "instock" &&
                                 CartItem.ProductQuantity <= _singleProduct.stock_quantity)
                        {
                            CartItem.ErrorMsg = "";
                            CartItem.InStock = true;
                        }
                    }
            }
            catch (Exception ex)
            {
                Crashes.TrackError(ex);
            }
        }

 private async Task VariableCheck()
        {
            //TODO if your seeing this, Your probably Developing on this now, if you're... Run! Honestly Run...
            try
            {
                foreach (var CartItem in _simpleCartlist)

                    if (_currentListItem == CartItem.PId)
                    {
                        if (_varProduct.stock_quantity == null)
                        {
                            _varProduct.stock_quantity = 999999;
                        }
                        if (CartItem.ProductQuantity == 0)
                        {
                            CartItem.ErrorMsg = "Item requires a Quantity of atleast 1" + System.Environment.NewLine + "Quantity Cannot be 0!";
                            await DisplayAlert("Oops", "Whe can't checkout nothing See if a products quantity isnt 0", "Ok");
                            return;
                        }
                        if (_varProduct.stock_quantity == 0 || _varProduct.stock_status == "outofstock" ||
                            CartItem.ProductQuantity > _varProduct.stock_quantity)
                        {
                            try
                            {
                                await Navigation.PopPopupAsync();

                            }
                            catch (Exception ex)
                            {
                                Crashes.TrackError(ex);
                            }

                            CartItem.InStock = false;
                            var AlertResult = await DisplayAlert("Not Enough Products",
                                $"There is this much Stock left: {_varProduct.stock_quantity} for {CartItem.ProductName} {_varProduct.attributes[2].option}",
                                "Back to Cart", "Keep Shopping");
                            CartItem.ErrorMsg = "Not Enough Products" + $"There is this much Stock left: {_varProduct.stock_quantity} for {CartItem.ProductName} {_varProduct.attributes[2].option}";
                            if (AlertResult)
                            {
                                CartItem.InStock = false;
                                CartItem.StockQuantity = Convert.ToInt32(_varProduct.stock_quantity);
                                _items = new ItemList(FullCart.CartList);
                                cartView.BeginRefresh();
                                CartItem.StockStatus = _varProduct.stock_status;
                                cartView.EndRefresh();
                                return;
                            }

                            var masterDetailPage = new Home();
                            masterDetailPage.Detail = new NavigationPage(new Home());
                            Application.Current.MainPage = masterDetailPage;
                        }
                        else if (_varProduct.stock_quantity != 0 && _varProduct.stock_status == "instock" &&
                                 CartItem.ProductQuantity <= _varProduct.stock_quantity)
                        {
                            CartItem.ErrorMsg = "";
                            CartItem.InStock = true;
                        }
                    }
            }
            catch (Exception ex)
            {
                Crashes.TrackError(ex);
            }
        }

 private readonly List<CartList> _simpleCartlist;
        private int _currentId;
        private int _currentListItem;
        private ItemList _items;
        private List<OrderLineItem> _orderlineitems;
        private bool _productBoughtOut;
        private List<string> _productnames;
        private Product _singleProduct;
        private bool _spamClick;
        private Variation _varProduct;


        private bool _loading;

        public bool loading
        {
            get => _loading;
            set
            {
                if (_loading == value) return;
                _loading = value;
                RaisePropertyChanged();

            }
        }


        private bool _running;

        public bool running
        {
            get => _running;
            set
            {
                if (_running == value) return;
                _running = value;
                RaisePropertyChanged();

            }
        }

The idea with spam-click is to avoid Double orders. I can definitely understand the frustration with all my if's. Any suggestions in regards to that would be appreciated.

Answer 1

Some quick remarks:

• Why do FullCart.CartList.Count <= 0 || !FullCart.CartList.Any()? That's the same check twice.

• Use proper variable names: var yx is meaningless.

• Do not hardcode these kind of things: new RestAPI("http://xxxxx/wp-json/wc/v2/", "xxxxxxxxxxxxxxxxxxxxxxxxx", "cs_xxxxxxxxxxxxxxxxxxxxxxxx");. API urls etc. should be in config files.

• Do not use magic strings: "on-hold" in var order = new Order { status = "on-hold", customer_id = Users.CId }; should really be an enum, or at least a centrally configured const string. Same for item.StockStatus == "instock" and other such items.

• Why is a single user represented by the object Users (as in Users.LoggedIn and Users.CId)?

• Do not leave out braces around large foreach sections etc.

• Where did you learn this: item.StockStatus is string stringValue && string.IsNullOrWhiteSpace(stringValue) || item.StockStatus == null? Why use this elaborate way when all you need is string.IsNullOrWhiteSpace(item.StockStatus)? (And again: StockStatus should be an enum.)

• Besides the meaningless variable name, what bothers me most about var a = Convert.ToInt32(item.ProductQuantity); is that ProductQuantity isn't an int to begin with.

• order.line_items: line_items should be Pascalcased. Moreover, property names and variable names etc. should only contain alphanumeric characters (exception: private class-wide variables can have an underscore at the start). Ditto product_id etc.

• Please get a native English speaker to spellcheck your messages. They often contain spelling errors and incorrect capitalization (e.g. "Order Cant be Placed", "Cart seem's to be empty, We cant checkout nothing",...). Also, "Woops" is not a word I expect to see in a e-commerce application.

• Split your methods into smaller ones. For instance, BeginCheckout() is 140 lines and mixes UI logic with business logic. Every call to Navigation is wrapped in a try...catch block which makes things hard to follow. To me that whole method should be a class of its own.

• Looking at SingleCheck() I'm baffled that you halt execution inside a foreach to wait for the user to respond in case of a lack of stock. Why not build a report instead and then bother the user only if one or more items cannot be fulfilled?

• foreach (var CartItem in _simpleCartlist): variables should be camelCased.

• Some of the business logic is just incomprehensible: if (_varProduct.stock_quantity == null) { _varProduct.stock_quantity = 999999; } How does that even make sense?

• How does this code even hang together? I don't see where _currentListItem is set. I don't see why you should need "6 seconds per item". I don't see calls to the APIs.

---

My main recommendation: split these large methods up into small ones, so the program flow becomes much clearer. Do not constantly bounce between business logic and UI calls.

Answer 2

Too many try/catch blocks, and it's redundant and misplaced. For instance Checkout_clicked method can be done this way :

private async void Checkout_clicked(object sender, EventArgs e)
{
    try
    {
        await Navigation.PushPopupAsync(new LoadingPopup());

        // what is happning here ? 
        // when Cart is NULL OR Does not have any elements OR Count is less than zero ? 
        if (FullCart.CartList == null || FullCart.CartList.Count <= 0 || !FullCart.CartList.Any())
        {
            var yx = await DisplayAlert("Whoops","Cart seem's to be empty, We cant checkout nothing", "Back to Cart", "Supplier");

            if (!yx)
            {
                var masterDetailPage = new Home();
                masterDetailPage.Detail = new NavigationPage(new Suppliers());
                Application.Current.MainPage = masterDetailPage;
            }
        }
        else
        {
            await BeginCheckout();
        }

        await Navigation.PopPopupAsync();

    }
    catch (Exception ex)
    {
        Crashes.TrackError(ex);
    }
}

same thing can applied to the other blocks. This is a must do modification as nested try/catch is a bad practice, ugly, and it would impact the performance. So, it must be avoided.

Secondly, when using if statements to validate, don't put your method validation in else clause. Take BeginCheckout as an example :

/*
    Part 1 : Validations 
*/

//You cant checkout if your not logged in There are no Guest Checkouts(I can But would rather not)
if (_spamClick)
{
    await DisplayAlert("Woops", "Your trying to order twice", "Ok");
    return; 
}

if (!Users.LoggedIn)
{
    // User is not logged in it should display this message immediately, and return it back to login screen or give the user the choice to go there     
    await DisplayAlert("Woops", "Please Login to check Out", "Login", "Home");
    return;
}

// this is okay
if (_orderlineitems == null) {  _orderlineitems = new List<OrderLineItem>(); }

// outside the loop since it's not releated to the loop 
if (!_simpleCartlist.Any(i => i.InStock == false) || _productBoughtOut) 
{ 
    await Navigation.PopPopupAsync(); 
    return;
}   

if(FullCart.CartList == null || _items.Items == null) { return; /* what should happen if the cart is null ? */ }


/*
    Part 2 : Process 
*/
foreach (var item in _simpleCartlist)
{   
    //if out of stock
    if (item.StockStatus != "instock" || !string.IsNullOrWhiteSpace(item.StockStatus)) { continue; } // skip this and go to the next item

    ... //rest of the loop 
    var a = Convert.ToInt32(item.ProductQuantity);
    if (item.VariationId <= 0) item.VariationId = item.PId;
    if (item.StockQuantity == 0)
    {
    _productBoughtOut = true;
    _productnames.Add(item.ProductName);
    }

    order.line_items = order.line_items ?? new List<OrderLineItem>();
    order.line_items.Add(new OrderLineItem
    { product_id = item.PId, variation_id = item.VariationId, quantity = a });  
}

_spamClick = true;
await wc.Order.Add(order);
Preferences.Clear("Cart");
Preferences.Remove("Cart");
FullCart.CartList.Clear();
_items.Items.Clear();
var masterDetailPage = new Home();
masterDetailPage.Detail = new NavigationPage(new Checkedout());
Application.Current.MainPage = masterDetailPage;

FOR DEMONSTRATION PURPOSE ONLY

Refactoring the if statements, to put the main validations at the top of the code, then go to the process. This way, the code would execute the validation part first, then if it passed this part it would go and execute the process part. This is a common used methodology since it's readable, easy to follow, and also follow the execution workflow.

The main idea is, whenever you see that the if condition would have a long executable logic or will include nested ifs, you should think of inverting it to make it simpler and extract the actual code outside it. Like the example above.

Also, in the example you will notice that I've used return; which is something I don't recommend to be used often in a work environment. But for testing and examples would be okay. The reason for that it would not handle anything, while you should handle every expected condition that happens in your logic to be more maintainable and understandable for other developers.

Another thing, braces exist for a reason. So, try to use them, as it would make things much easier to follow and maintainable. Even in Visual Studio, with braces, you could collapse or expand any part you want in the source.