1
\$\begingroup\$

The following class is a wrapper for Microsoft.Win32.SaveFileDialog. I've implemented the usage of Microsoft.Win32.SaveFileDialog this way because I'm using the ISaveFileDialog interface as a dependency throughout my code base.

public class SaveFileDialog : ISaveFileDialog
{
    public bool Save(string content, string suggestedFileExtension = null, string suggestedFileExtensionName = null, string suggestedFileName = null)
    {
        string filter = "All Files|*.*";

        if (suggestedFileExtension != null)
        {
            filter = $"{suggestedFileExtensionName ?? string.Empty}|*{suggestedFileExtension}|" + filter;
        }

        Microsoft.Win32.SaveFileDialog dlg = new Microsoft.Win32.SaveFileDialog
        {
            FileName = suggestedFileName ?? string.Empty,
            DefaultExt = suggestedFileExtension ?? string.Empty,
            Filter = filter
        };

        switch (dlg.ShowDialog())
        {
            case true:
                return WriteFile(dlg.FileName, content);
            default:
                return false;
        }
    }

    private bool WriteFile(string filePath, string content)
    {
        try
        {
            File.WriteAllText(filePath, content);
            return true;
        }
        catch (PathTooLongException)
        {
            return false;
        }
        catch (DirectoryNotFoundException)
        {
            return false;
        }
        catch (IOException)
        {
            return false;
        }
        catch (UnauthorizedAccessException)
        {
            return false;
        }
        catch (System.Security.SecurityException)
        {
            return false;
        }
    }
}

While this works, I'm struggling to refactor this class to allow for unit testing as currently there's no way to mock/stub the Microsoft.Win32.SaveFileDialog - have I missed a trick here? Or is there genuinely no nice way to unit test this and I should be relying on integration tests?


I know that I could refactor the catch blocks to either be

catch (Exception)
{
    return false;
}

Or even to be

catch (Exception ex) when (ex is PathTooLongException
                        || ex is DirectoryNotFoundException
                        ........ )
{
    return false;
}

But I would like to keep them the way that they are because I would like to avoid catching all exceptions and I'm using a Visual Studio plugin to help me manage exceptions and it complains when using the second option.

Thanks for any help!

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

unit tests

You can use fakes (stubs vs shims) to mock third-party classes. It is not as lightweight as your regular mocks, but it does the job.


exception handling

catch (Exception)
{
    return false;
}

Why don't you do something like Microsoft tends to do often in their framework? You make some internal convenience method IsCriticalException that checks whether to rethrow the exception or swallow it.

catch (Exception ex)
{
    if (IsCriticalException(ex)) throw;
    return false;
}

clean code

Use var when you can.

Microsoft.Win32.SaveFileDialog dlg = new Microsoft.Win32.SaveFileDialog

 var dlg = new Microsoft.Win32.SaveFileDialog
\$\endgroup\$
2
  • \$\begingroup\$ Why should I use var wherever I can? \$\endgroup\$
    – Toby Smith
    Commented May 30, 2019 at 11:08
  • 1
    \$\begingroup\$ @TobySmith To avoid redundant code and improve readability. It's a mere semantic optimization. stackoverflow.com/questions/209199/… \$\endgroup\$
    – dfhwze
    Commented May 30, 2019 at 11:15
0
\$\begingroup\$

I think you are unit testing the wrong code. You should be looking to unit test your code and not Microsofts code. It's a good place to write interfaces around their code for testing so you can switch out MS code with your own test implementation. You didn't show what your interface looked like but I would suggest that if you want to replace SaveFileDialog then you should have an interface just for that and not also have SaveFileDialog also write the file.

Something along the lines of

public interface ISaveFileDialog
{
    string GetFileName(string suggestedFileExtension = null, string suggestedFileExtensionName = null, string suggestedFileName = null);
}

public class SaveFileDialog : ISaveFileDialog
{
    public string GetFileName(string suggestedFileExtension = null, string suggestedFileExtensionName = null, string suggestedFileName = null)
    {
        string filter = "All Files|*.*";

        if (suggestedFileExtension != null)
        {
            filter = $"{suggestedFileExtensionName ?? string.Empty}|*{suggestedFileExtension}|" + filter;
        }

        Microsoft.Win32.SaveFileDialog dlg = new Microsoft.Win32.SaveFileDialog
        {
            FileName = suggestedFileName ?? string.Empty,
            DefaultExt = suggestedFileExtension ?? string.Empty,
            Filter = filter
        };

        switch (dlg.ShowDialog())
        {
            case true:
                return dlg.FileName;
            default:
                return null;
        }
    }
}

public interface IWriteFile
{
    bool WriteText(string fileName, string content);
}

public class WriteFile : IWriteFile
{
    public bool WriteText(string fileName, string content)
    {
        try
        {
            File.WriteAllText(fileName, content);
        }
         // Just a simple catch to show example
        catch (Exception ex)
        {
            return false;
        }
        return true;
    }
}

public class SaveFile
{
    private readonly ISaveFileDialog saveFileDialog;
    private readonly IWriteFile writer;
    public SaveFile(ISaveFileDialog saveFileDialog, IWriteFile writer)
    {
        this.saveFileDialog = saveFileDialog;
        this.writer = writer;
    }

    public bool Save(string content, string suggestedFileExtension = null, string suggestedFileExtensionName = null, string suggestedFileName = null)
    {
        var fileName = saveFileDialog.GetFileName(suggestedFileExtension, suggestedFileExtensionName, suggestedFileName);
        if (fileName == null)
        {
            return false;
        }

        return writer.WriteText(fileName, content);

    }
}

Now we wrapped MS code in interfaces that we want to test. For unit test you can now have a mock of both ISaveFileDialog and IWriteFile.

Unit test against SaveFile could be the following:

  1. Verify that if GetFileName return null that WriteText never got hit
  2. GetFileName return a string that WriteText did get hit
  3. SaveFile returns true if WriteText returns true
  4. SaveFile returns false if WRiteText returns false
  5. if GetFileName returns null that SaveFile returns false
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.