Andreas Panteli
Andreas Panteli

Reputation: 47

Refactoring block of if statements in c#

I have a function that i want to refactor and i am a bit stuck.

    public async Task<IActionResult> SaveFileContents(string filename, string connectionId, string fileContents, string repositoryName, string branch, string button)
    {
        var repository = ViewModel.Products.FirstOrDefault(x => x.RepositoryName == repositoryName);
        var cachedFilename = MemoryCache.Get<string>(connectionId);

        if (button=="btn-config")
        {
            var testsFolderPath = repository != null ? repository.TestsFolderPath : "";
            var testsDirectory = GetTestsDirectory(repositoryName, branch, testsFolderPath);
            if (cachedFilename.Contains(filename))
            {
                var fullPath = string.Empty;
                var configPath = GetConfigPath(repositoryName, branch);

                if (Path.GetFileName(configPath) == filename)
                {
                    fullPath = configPath;
                }
                else
                {
                    fullPath = Path.Combine(testsDirectory, filename);
                }

                await IOHelper.WriteContentsToFileAsync(fullPath, fileContents);
            }
        }
        else
        {
            var stepFilePath = repository != null ? repository.StepFilePath : "";
            var stepFileDirectory = GetStepFile(repositoryName, branch, stepFilePath);
            if (cachedFilename.Contains(filename))
            {
                var fullPath = string.Empty;
                var stepFile = GetStepFile(repositoryName, branch, stepFilePath);

                if (Path.GetFileName(stepFile) == filename)
                {
                    fullPath = stepFileDirectory;
                }
                else
                {
                    fullPath = Path.Combine(stepFilePath, filename);
                }
                await IOHelper.WriteContentsToFileAsync(fullPath, fileContents);
            }
        }
        return Ok();
    }

As you can observe the majority of the lines do similar things. I managed to refactor it to

        var repository = ViewModel.Products.FirstOrDefault(x => x.RepositoryName == repositoryName);
        var cachedFilename = MemoryCache.Get<string>(connectionId);

        if (button == "btn-config")
        {
            var testsFolderPath = repository != null ? repository.TestsFolderPath : "";
            var testsDirectory = GetTestsDirectory(repositoryName, branch, testsFolderPath);
            var configPath = GetConfigPath(repositoryName, branch);
        }
        else
        {
            var testsDirectory = repository != null ? repository.StepFilePath : "";
            var configPath = GetStepFile(repositoryName, branch, testsDirectory);
            var stepFile = GetStepFile(repositoryName, branch, testsDirectory);
        }

        if (cachedFilename.Contains(filename))
        {
            var fullPath = string.Empty;
            if (Path.GetFileName(configPath) == filename)
            {
                fullPath = configPath;
            }
            else
            {
                fullPath = Path.Combine(testsDirectory, filename);
            }
            await IOHelper.WriteContentsToFileAsync(fullPath, fileContents);
        }
        return Ok();
    }

but the values in the second if statement show that they do not exist in the current context. Any guidelines and tips are greatly appreciated.

Upvotes: 0

Views: 73

Answers (2)

Vijayanath Viswanathan
Vijayanath Viswanathan

Reputation: 8571

Further refactored your refactored code,

var repository = ViewModel.Products.FirstOrDefault(x => x.RepositoryName == repositoryName);
        var cachedFilename = MemoryCache.Get<string>(connectionId);

        if (button == "btn-config")
        {
            var testsFolderPath = repository != null ? repository.TestsFolderPath : "";
            var testsDirectory = GetTestsDirectory(repositoryName, branch, testsFolderPath);
            var configPath = GetConfigPath(repositoryName, branch);
        }
        else{
            var testsDirectory = repository != null ? repository.StepFilePath : "";
            var configPath = GetStepFile(repositoryName, branch, testsDirectory);
            var stepFile = GetStepFile(repositoryName, branch, testsDirectory);
           }
     string fullPath = string.empty;

        if (cachedFilename.Contains(filename) && Path.GetFileName(configPath) == filename)
        {
            fullPath = configPath;

        }
        else{
        fullPath = Path.Combine(testsDirectory, filename);
          }
        await IOHelper.WriteContentsToFileAsync(fullPath, fileContents);

        return Ok();

Grouped two if checks together

Upvotes: 1

Frauke
Frauke

Reputation: 1582

The problem with your refactored code is that several of your variables go out of scope before you try to use them. Right now, the following are declared within the if...else block: testsFolderPath, testsDirectory and configPath. This means that when you hit your second if block, these variables simply do not exist any longer.

You need to declare them in a scope that is accessible in all places where you use them, like so:

    string testsFolderPath = String.Empty;
    string testsDirectory = String.Empty;
    string configPath = String.Empty;

    if (button == "btn-config")
    {
        testsFolderPath = repository != null ? repository.TestsFolderPath : "";
        testsDirectory = GetTestsDirectory(repositoryName, branch, testsFolderPath);
        configPath = GetConfigPath(repositoryName, branch);
    }
    else
    {
        testsDirectory = repository != null ? repository.StepFilePath : "";
        configPath = GetStepFile(repositoryName, branch, testsDirectory);
        stepFile = GetStepFile(repositoryName, branch, testsDirectory);
    }

    if (cachedFilename.Contains(filename))
    {
        var fullPath = string.Empty;
        if (Path.GetFileName(configPath) == filename)
        {
            fullPath = configPath;
        }
        else
        {
            fullPath = Path.Combine(testsDirectory, filename);
        }
        await IOHelper.WriteContentsToFileAsync(fullPath, fileContents);
    }
    return Ok();

I have made assumptions about the type of testsFolderPath, testsDirectory and configPath, since you don't explicitly declare them as a particular type.

Upvotes: 0

Related Questions