Method if condition logic

Question

This logic checks accessGroup and if accessGroup is equal to "Admin" then it only checks if result.Admin or baccess is true but if accessGroup is anthing else it will need to check two other objects result.Admin == true || result.PowerUser. Is there any other way to do this if condition?

if (accessGroup == "Admin")
{
 if (baccess == true || result.Admin == true)
 {
 var FileInfo = GetFile(fileManagerGuidId);
 if (FileInfo != null)
 {
 FileManagerLog _filemanagerLog = new FileManagerLog();
 _filemanagerLog.CustomerId =Request.Cookies["customerid"] != null ? Convert.ToInt32(Request.Cookies["customerid"].Value) : 0;
 _filemanagerLog.FileManagerGuid = new Guid(fileManagerGuidId);
 SaveFileManagerLog(_filemanagerLog);
 byte[] fileBytes = FileInfo.FileData;
 return File(fileBytes, System.Net.Mime.MediaTypeNames.Application.Octet, FileInfo.FileName);
 }
 else
 {
 return null;
 }
 }
}
else
{
 if (baccess == true || result.Admin == true || result.PowerUser)
 {
 var FileInfo = GetFile(fileManagerGuidId);
 if (FileInfo != null)
 {
 FileManagerLog _filemanagerLog = new FileManagerLog();
 _filemanagerLog.CustomerId =Request.Cookies["customerid"] != null ? Convert.ToInt32(Request.Cookies["customerid"].Value) : 0;
 _filemanagerLog.FileManagerGuid = new Guid(fileManagerGuidId);
 SaveFileManagerLog(_filemanagerLog);
 byte[] fileBytes = FileInfo.FileData;
 return File(fileBytes, System.Net.Mime.MediaTypeNames.Application.Octet, FileInfo.FileName);
 }
 else
 {
 return null;
 }
 }
}

• Dmitry: if (baccess || (accessGroup == "Admin" ? result.Admin : (result.Admin || result.PowerUser)))
• Heslacher: Is this your real code? If yes, which I can't believe, both branches will execute the same code hence you could just skip the if..else.
• BCdotWEB: The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?.
• Flater: The issue with indentation is significantly compounding the readability and subsequently a reader's understanding of the intended logic. This is a good case example of why deeply nested if structures are bad for readability.
• MikeT: on a side note Request.Cookies["customerid"] != null ? Convert.ToInt32(Request.Cookies["customerid"].Value) : 0; can be much simplified to Convert.ToInt32(Request.Cookies["customerid"]?.Value ?? 0);

Answer 1

here is a clearer view of the current code :

if (accessGroup == "Admin")
{
 if (baccess == true || result.Admin == true)
 {
 var FileInfo = GetFile(fileManagerGuidId);
 if (FileInfo != null)
 {
 // return File(...);
 }
 else
 {
 return null;
 }
 }
}
else
{
 if (baccess == true || result.Admin == true || result.PowerUser)
 {
 var FileInfo = GetFile(fileManagerGuidId);
 
 if (FileInfo != null)
 {
 // return File(...);
 }
 else
 {
 return null;
 }
 }
}

Can't you see both return the same file ? If this works very well with current project, then you can simplify it to :

if (accessGroup == "Admin" || baccess == true || result.Admin == true || result.PowerUser)
{
 var FileInfo = GetFile(fileManagerGuidId);
 if (FileInfo != null)
 {
 // return File(...);
 }
 
}
else
{
 return null; 
}

if your actual file log is different than the code, and it has different logic based on the conditions above, then you can translate it to this :

if(baccess == false && result.Admin == false)
{
 return null;
}
var FileInfo = GetFile(fileManagerGuidId);
if(FileInfo == null)
{
 return null;
}
if(accessGroup == "Admin") 
{
 // Admin file logic 
 // return File(...); 
}
if(result.PowerUser)
{
 // PowerUser file logic 
 // return File(...);
}

• MikeT: i made the same mistake when i first read his code accessGroup != "Admin" And's with powerUsers not Or's

Answer 2

this looks like it will have the same effect

if (baccess || result.Admin || (accessGroup != "Admin" && Result.PowerUser))

though i suspect what you mean to do is AND (&&)

if (baccess && (result.Admin || (accessGroup != "Admin" && result.PowerUser))

ie if they don't have access or the right access level don't run the code