Can anyone tell me why the maintainability index is only 40 for this code?

290 views Asked by At

I can't figure out why the maintainability index (as calculated in Visual Studio) for this method is only 40, I literally have to remove almost all the lines except the first two to get above 60:

    public void getNewPasswordDetails(Int32 newPasswordId)
    {

        int UserId = HttpContext.Current.User.Identity.GetUserId().ToInt();
        bool userIsAdmin = HttpContext.Current.User.IsInRole("Administrator");

        //get a list of userIds that have UserPassword records for this password
        var UserIDList = DatabaseContext.UserPasswords.Where(up => up.PasswordId == newPasswordId).Select(up => up.Id).ToList();

        //Retrive the password -if the user has access
        Password newPassword = DatabaseContext.Passwords
                                                        .Include("Creator")
                                                        .Where(pass => !pass.Deleted
                                                        && (
                                                            (UserIDList.Contains(UserId))
                                                         || (userIsAdmin && ApplicationSettings.Default.AdminsHaveAccessToAllPasswords)
                                                         || pass.Creator_Id == UserId)
                                                            )
                                                            .Include(p => p.Parent_UserPasswords.Select(up => up.UserPasswordUser))
                                                            .SingleOrDefault(p => p.PasswordId == newPasswordId);



        if (newPassword != null)
        {
            //map new password to display view model
            AutoMapper.Mapper.CreateMap<Password, PasswordItem>();
            PasswordItem returnPasswordViewItem = AutoMapper.Mapper.Map<PasswordItem>(newPassword);

            //generate a string based view of the new category
            string passwordPartialView = RenderViewContent.RenderViewToString("Password", "_PasswordItem", returnPasswordViewItem);

            //broadcast the new password details
            PushNotifications.sendAddedPasswordDetails(passwordPartialView, returnPasswordViewItem.Parent_CategoryId, returnPasswordViewItem.PasswordId);
        }
        else
        {
            //we dont have access any more, so tell UI to remove the password
            PushNotifications.sendRemovePasswordAccess(new PasswordDelete()
                                                                { 
                                                                     PasswordId = newPasswordId
                                                                });
        }

    }
1

There are 1 answers

2
ΩmegaMan On

The more complex code is, the harder it is to maintain. Right? So let us look at a complex section of code which was presented. I will examine it as if I was a developer looking at the code for the first time:

DatabaseContext.Passwords
               .Include("Creator")
               .Where(pass => !pass.Deleted
                          && ((UserIDList.Contains(UserId))     // Why Check #1
                               || (
                                   userIsAdmin                // Why Check #2
                                   &&                        // Why Check #3
                                   ApplicationSettings.Default.AdminsHaveAccessToAllPasswords
                                  )
                              || pass.Creator_Id == UserId) 
                             )
               .Include(p => p.Parent_UserPasswords.Select(up => up.UserPasswordUser))
               .SingleOrDefault(p => p.PasswordId == newPasswordId);

Before entering the loop one already knows these state facts:

  1. (UserIDList.Contains(UserId) as Why Check #1
  2. userIsAdmin as why Check #2
  3. (userIsAdmin && ApplicationSettings.Default.AdminsHaveAccessToAllPasswords) as why check #3

Yet the developer has relegated those checks to occur for every password in Passwords. Why? Isn't there a better way of expressing that ?

Complexity occurs due to the application (coding) of the program logic, for each logical fork in the determination of the business logic directly adds to the complexity of the code and subsequently future maintainability; hence the rating received.

Of course one has certain complexities in code just by its nature, that is going to happen and should be expected. The question is, can that complexity be minimized to the point where maintenance of the code can be better achieved.