Code efficiency and readability ef core

Question

More advise this anything their has to be a better way of handling this code I feel am doing to much just to set those four vars to false.

 private async void SwipeItem_Tap(object sender, SwipeItemTapEventArgs e)
    {
        var item = e.Item as Session;
        var action =await DisplayAlert(Constants.AppName, "You are about to delete this sesison. This action will also remove any players linked to this session their work out history will not be touched. Are you sure?", "YES", "NO");
        if (action != false)
        {
            if (item != null)
            {
                var session = api.GetSessionById(item.Id);
                session.IsDeleted = true;
                session.IsActive = false;

                await api.UpdateSession(session);
                await DisplayAlert(Constants.AppName, "Session Updated", null, "Ok");
                BindGrid();

                var sessionplayers =await api.GetAllSessionPlayersBySessionId(item.Id);
                foreach (var player in sessionplayers)
                {
                    SessionPlayer playerSession = new SessionPlayer();
                    playerSession = player;
                    playerSession.IsActive = false;
                    playerSession.IsDeleted = true;
                    playerSession.SessionId = 0;
                    await api.UpdateSessionPlayers(playerSession);
                    await DisplayAlert(Constants.AppName, "Any Sessions the player belonged to have been removed", null, "Ok");

                }
                await DisplayAlert(Constants.AppName, "Session Removal Completed", null, "Ok");
            }
        }else
        {
            await DisplayAlert(Constants.AppName, "Session Not Updated", null, "Ok");

        }

    }`

Answer 1

I try to program defensively as it tends to push all the "unhappy" code paths to the start of your method, then you know that the remainder of the method is the "happy" path.

It reduces the amount of indenting/branching in the main body of the method, and typically means that you handle the error-cases of your method as a forethought rather than an afterthought (as I've noticed in my older projects when I overuse else statements.)

So for example I'd be tempted to restructure it slightly:

if (!action) {
    await DisplayAlert(Constants.AppName, "Session Not Updated", null, "Ok");
    return;
}
if (item == null) {
    return;
}

var session = api.GetSessionById(item.Id);
session.IsDeleted = true;
session.IsActive = false;

await api.UpdateSession(session);
await DisplayAlert(Constants.AppName, "Session Updated", null, "Ok");

BindGrid();

var sessionplayers = await api.GetAllSessionPlayersBySessionId(item.Id);

foreach (var player in sessionplayers) {
    SessionPlayer playerSession = new SessionPlayer();
    playerSession = player;
    playerSession.IsActive = false;
    playerSession.IsDeleted = true;
    playerSession.SessionId = 0;
    await api.UpdateSessionPlayers(playerSession);
    await DisplayAlert(Constants.AppName, "Any Sessions the player belonged to have been removed", null, "Ok");
}

await DisplayAlert(Constants.AppName, "Session Removal Completed", null, "Ok");

(notice that the bulk of the method is flat to the left hand side without any branching logic to follow)

This is just my preference though, so you're best off getting a consensus of what's easiest to read from your team.

Answer 2

Some observations below.

if (action != false) can be simplified to if(!action)

also you can merge those two ifs to one.

if(!action && item != null)
{
// Your code here

}
else
{
//
}