Preem
Preem

Reputation: 47

Cleaning up nested if statements to be more readable?

I'm writing a project, and the part I'm doing now is getting arrow shaped real fast. How can I remove the nested if statements, but still have the same behaviour?

The code below might not look so bad now, but I'm planning on refactoring to include more methods.

public async Task FirstDiffTestAsync()
{
    string folderDir = "../../../";
    string correctReportDir = folderDir + "Reports To Compare/Testing - Copy.pdf";
    string OptyNumber = "122906";

    //Making a POST call to generate report
    string result = ReportGeneration(OptyNumber).Result;
    Response reportResponse = JsonConvert.DeserializeObject<Response>(result);
    string newURL = reportResponse.documentUrl;

    //Logging the Response to a text file for tracking purposes
    await File.WriteAllTextAsync(Context.TestRunDirectory + "/REST_Response.txt", result);

    using (StreamWriter w = File.AppendText(Context.TestDir + "/../log.txt"))
    {
        //Checking if the Integration failed
        if (reportResponse.Error == null)
        {
            //now we have the url, reading in the pdf reports
            List<string> Files = new List<string> { correctReportDir, newURL };
            List<string> parsedText = PdfToParsedText(Files);

            DiffPaneModel diff = InlineDiffBuilder.Diff(parsedText[0], parsedText[1]);

            // DiffReport is a customised object
            DiffReport diffReport = new DiffReport(correctReportDir, newURL);
            diffReport.RunDiffReport(diff);

            //In-test Logging
            string indent = "\n      - ";
            string logMsg = $"{indent}Opty Number: {OptyNumber}{indent}Activity Number: {reportResponse.ActivityNumber}{indent}File Name: {reportResponse.FileName}";
            if (diffReport.totalDiff != 0)
            {
                await File.WriteAllTextAsync(Context.TestRunDirectory + "/DiffReport.html", diffReport.htmlDiffHeader + diffReport.htmlDiffBody);
                logMsg += $"{indent}Different lines: {diffReport.insertCounter} Inserted, {diffReport.deleteCounter} Deleted";
            }
            LogTesting(logMsg, w);

            //Writing HTML report conditionally
            if (diffReport.totalDiff != 0)
            {
                await File.WriteAllTextAsync(Context.TestRunDirectory + "/DiffReport.html", diffReport.htmlDiffHeader + diffReport.htmlDiffBody);
            }
            Assert.IsTrue(diffReport.insertCounter + diffReport.deleteCounter == 0);
        }
        else
        {
            LogTesting($" Integration Failed: {reportResponse.Error}", w);
            Assert.IsNull(reportResponse.Error);
        }
    }
}

Upvotes: 2

Views: 1128

Answers (2)

Kent Kostelac
Kent Kostelac

Reputation: 2446

I wouldn't consider this as having an excessive amount of nested if-statements. It is fine as is. Otherwise you could do the following (also suggested by @Caius Jard):

public async Task FirstDiffTestAsync()
{
    string folderDir = "../../../";
    string correctReportDir = folderDir + "Reports To Compare/Testing - Copy.pdf";
    string OptyNumber = "122906";

    //Making a POST call to generate report
    string result = ReportGeneration(OptyNumber).Result;
    Response reportResponse = JsonConvert.DeserializeObject<Response>(result);
    
    //Checking if the Integration failed
    if (reportResponse.Error != null)
    {
        LogTesting($" Integration Failed: {reportResponse.Error}", w);
        Assert.IsNull(reportResponse.Error);
        return;
    }

    string newURL = reportResponse.documentUrl;

    //Logging the Response to a text file for tracking purposes
    await File.WriteAllTextAsync(Context.TestRunDirectory + "/REST_Response.txt", result);

    using (StreamWriter w = File.AppendText(Context.TestDir + "/../log.txt"))
    {
        //now we have the url, reading in the pdf reports
        List<string> Files = new List<string> { correctReportDir, newURL };
        List<string> parsedText = PdfToParsedText(Files);

        DiffPaneModel diff = InlineDiffBuilder.Diff(parsedText[0], parsedText[1]);

        // DiffReport is a customised object
        DiffReport diffReport = new DiffReport(correctReportDir, newURL);
        diffReport.RunDiffReport(diff);

        //In-test Logging
        string indent = "\n      - ";
        string logMsg = $"{indent}Opty Number: {OptyNumber}{indent}Activity Number: {reportResponse.ActivityNumber}{indent}File Name: {reportResponse.FileName}";
        if (diffReport.totalDiff != 0)
        {
            await File.WriteAllTextAsync(Context.TestRunDirectory + "/DiffReport.html", diffReport.htmlDiffHeader + diffReport.htmlDiffBody);
            logMsg += $"{indent}Different lines: {diffReport.insertCounter} Inserted, {diffReport.deleteCounter} Deleted";
        }
        LogTesting(logMsg, w);

        //Writing HTML report conditionally
        if (diffReport.totalDiff != 0)
        {
            await File.WriteAllTextAsync(Context.TestRunDirectory + "/DiffReport.html", diffReport.htmlDiffHeader + diffReport.htmlDiffBody);
        }
        Assert.IsTrue(diffReport.insertCounter + diffReport.deleteCounter == 0);
    }
}

Upvotes: 0

ofka27
ofka27

Reputation: 11

As mentioned in the comment, the indentation level is fine for now, but its always better to minimize when possible, especially when you are repeating same blocks of code.

The best way to do this is to write a separate function that contains that block of code and then call that function instead of the nested if statements.

In your case it would be something like this:

private async void checkTotalDiff(diffReport) {
      ...
}

You could pass anything you might need in the parameters. This way in your main code, you could replace the if statements with checkTotalDiff(diffReport) and save the return (if any) to a variable.

Also note I used void for return but you could change the type depending on what the function returns.

Upvotes: 1

Related Questions