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
});
}
}
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:
Before entering the loop one already knows these state facts:
(UserIDList.Contains(UserId)
as Why Check #1userIsAdmin
as why Check #2(userIsAdmin && ApplicationSettings.Default.AdminsHaveAccessToAllPasswords)
as why check #3Yet 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.