3
\$\begingroup\$

I have a WinForms project in which I am trying to implement the Passive View MVP pattern (meaning no business logic in my Views). Each form is a concrete View with an IView interface to which a Presenter is connected.

I do think i should handle some UI related logic in my View because I am otherwise adding needless complexity to try to handle this in the Presenter. Therefore, I have created a simple function in my View class which validates the result from the file dialog. I did not put this logic in the event handler of the button press because I wanted to avoid duplicate code since I have three of these buttons.

Please let me know if you think this is an appropriate way to implement this. I have posted the function, and the event (I have three button_Click events that each use this function) that utilizes this function below.

Button event that uses the GetFileName function:

// Gets the filename and fires the compliance standard adding event.
private void newComplianceStandardButton_Click(object sender, EventArgs e)
{
    string fileName = GetFileName();

    if (fileName != null)
    {
        AddingComplianceStandard?.Invoke(this, fileName);
    }

    Close();
}

Function that checks the dialog result and returns the filename:

// Opens the openfile dialog and checks the result. If the result is OK, the form is closed and the filename is returned.
public string GetFileName()
{
    DialogResult result = openFileDialog1.ShowDialog();
    string fileName = openFileDialog1.FileName;

    if (result == DialogResult.OK &&
        (Path.GetExtension(openFileDialog1.FileName) == ".txt" || Path.GetExtension(openFileDialog1.FileName) == ".csv"))
    {
        Close();
        return fileName;
    }
    else
    {
        MessageBox.Show("You have selected a file with an illegal extension. Please try again and select a *.txt or a *.csv file");
        return null;
    }
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

In the big if you have the following:

Path.GetExtension(openFileDialog1.FileName) == ".txt" || Path.GetExtension(openFileDialog1.FileName) == ".csv")

Why? You already put openFileDialog1.FileName in a fileName variable. Just use that.

The next thing I would do is define a list of valid extensions:

public readonly string[] ValidExtensions => new string[] { "txt", "csv" }

Then simply:

if (result == DialogResult.OK && ValidExtensions.Contains(Path.GetExtension(fileName).Substring(1)))

We use the .Substring(1) to remove the period.

That way, if you add support for a different file type, you just add it to ValidExtensions.

\$\endgroup\$
0

Your Answer

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

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