Drag and drop single files on different targets for different purposes

Question

I have three text fields (DevExpress ButtonEdits set to read only) on a Windows Forms app. They look like this:

They're used by dragging and dropping a single file on each field.

• If a dropped file belongs on a particular field (correct file type with legal contents), the file is parsed and its contents are used to populate the corresponding place in the applications object model. The text field changes to reflect the location of the loaded file.
• If a dropped file does not belong on a particular text field (illegal, incorrect, or corrupt contents), an alert message is shown and the application is unaffected. The text field remains blank/unchanged.

I currently achieve this with the following code:

public partial class MainForm : DevExpress.XtraEditors.XtraForm
{

    private void MainForm_Load(object sender, EventArgs e)
    {
        SetDragDropHandlers();
    }

    // Here the drag drop and drag enter handlers are assigned at most once to each field.
    private void SetDragDropHandlers()
    {

        // REVIEWER: SEE REPETITION 1
        /////////////////////////////

        // Using -= then += on a handler ensures only one is ever added.

        this.TextCalibrationFile.AllowDrop = true;
        this.TextCalibrationFile.DragEnter -= new DragEventHandler(ActivateIfSingleFileDrag);
        this.TextCalibrationFile.DragEnter += new DragEventHandler(ActivateIfSingleFileDrag);
        this.TextCalibrationFile.DragDrop -= new DragEventHandler(TryExtractCalibration);
        this.TextCalibrationFile.DragDrop += new DragEventHandler(TryExtractCalibration);

        this.TextWhiteBalance.AllowDrop = true;
        this.TextWhiteBalance.DragEnter -= new DragEventHandler(ActivateIfSingleFileDrag);
        this.TextWhiteBalance.DragEnter += new DragEventHandler(ActivateIfSingleFileDrag);
        this.TextWhiteBalance.DragDrop -= new DragEventHandler(TryExtractWhiteBalance);
        this.TextWhiteBalance.DragDrop += new DragEventHandler(TryExtractWhiteBalance);

        this.TextDarkBalance.AllowDrop = true;
        this.TextDarkBalance.DragEnter -= new DragEventHandler(ActivateIfSingleFileDrag);
        this.TextDarkBalance.DragEnter += new DragEventHandler(ActivateIfSingleFileDrag);
        this.TextDarkBalance.DragDrop -= new DragEventHandler(TryExtractDarkBalance);
        this.TextDarkBalance.DragDrop += new DragEventHandler(TryExtractDarkBalance);
    }

    // This is registered to the DragEnter event to ensure a single file is being dragged.
    private void ActivateIfSingleFileDrag(object sender, DragEventArgs evt)
    {
        if (evt.Data.GetDataPresent(DataFormats.FileDrop) && ((string[])evt.Data.GetData(DataFormats.FileDrop)).Length == 1)
            evt.Effect = DragDropEffects.Copy;
    }

    // This is registered to the calibration-specific DragDrop event to load a calibration into the app.
    private void TryExtractCalibration(object sender, DragEventArgs evt)
    {
        string[] files = (string[])evt.Data.GetData(DataFormats.FileDrop);
        if (files.Length > 0)
        {
            try
            {
                // TODO attempt to load a ChannelSetCalibration from file contents


                // load was successful, so set the UI text to the source file's path
                ((ButtonEdit)sender).EditValue = files[0];
            } catch (ArgumentException ex)
            {
                DisplayAlert("Could not load Calibration from file!\n" + ex.Message);
            }
        }
    }

    // This is registered to the white balance-specific DragDrop event to load a white balance into the app.
    private void TryExtractWhiteBalance(object sender, DragEventArgs evt)
    {

        // REVIEWER: SEE REPETITION 2
        /////////////////////////////

        string[] files = (string[])evt.Data.GetData(DataFormats.FileDrop);
        if (files.Length > 0)
        {
            try
            {
                // TODO attempt to load a white balance ImageCube from file contents


                // load was successful, so set the UI text to the source file's path
                ((ButtonEdit)sender).EditValue = files[0];
            }
            catch (ArgumentException ex)
            {
                DisplayAlert("Could not load White Balance from file!\n" + ex.Message);
            }
        }
    }

    // This is registered to the dark balance-specific DragDrop event to load a dark balance into the app.
    private void TryExtractDarkBalance(object sender, DragEventArgs evt)
    {

        // REVIEWER: SEE REPETITION 2
        /////////////////////////////

        string[] files = (string[])evt.Data.GetData(DataFormats.FileDrop);
        if (files.Length > 0)
        {
            try
            {
                // TODO attempt to load a dark balance ImageCube from file contents


                // load was successful, so set the UI text to the source file's path
                ((ButtonEdit)sender).EditValue = files[0];
            }
            catch (ArgumentException ex)
            {
                DisplayAlert("Could not load Dark Balance from file!\n" + ex.Message);
            }
        }
    }

    private void DisplayAlert(string message)
    {
        var alertInfo = new AlertInfo("Warning", message);
        this.AlertWarning.Show(this, alertInfo);
    }

    public MainForm()
    {
        Load += MainForm_Load;
        InitializeComponent();
    }
}

---

This works just fine, but my event handling logic looks like it should be much more DRY. Or, to summarize...

My handlers are all wet.

Repetition 1

The three chunks that look like

this.TextCalibrationFile.AllowDrop = true;
this.TextCalibrationFile.DragEnter -= new DragEventHandler(ActivateIfSingleFileDrag);
this.TextCalibrationFile.DragEnter += new DragEventHandler(ActivateIfSingleFileDrag);
this.TextCalibrationFile.DragDrop -= new DragEventHandler(TryExtractCalibration);
this.TextCalibrationFile.DragDrop += new DragEventHandler(TryExtractCalibration);

only differ by the parameter to the DragDrop handler (TryExtractCalibration here). I say my handlers are all wet because these three blocks should be DRYed out into three method calls that look something like

RegisterFileDropHandler(this.TextCalibrationFile, TryExtractCalibration);
RegisterFileDropHandler(this.TextWhiteBalance, TryExtractWhiteBalance);
RegisterFileDropHandler(this.TextDarkBalance, TryExtractDarkBalance);

but I don't know haw to pass methods like parameters in C#!

Repetition 2

Another related problem is the similarity between the three event handler methods themselves. They all contain a body like

string[] files = (string[])evt.Data.GetData(DataFormats.FileDrop);
if (files.Length > 0)
{
    try
    {
        // TODO attempt to load a ChannelSetCalibration from file contents


        // load was successful, so set the UI text to the source file's path
        ((ButtonEdit)sender).EditValue = files[0];
    } catch (ArgumentException ex)
    {
        DisplayAlert("Could not load Calibration from file!\n" + ex.Message);
    }
}

They only differ by their try block bodies and their alert messages. Passing an alert message string into a method is simple enough, but if I try adding one

private void TryExtractCalibration(object sender, DragEventArgs evt, string errorMsg)

then I can no longer use the method to create a DragEventHandler. That is, new DragEventHandler(TryExtractCalibration); becomes illegal.

Even if I knew how to resolve that, the try block body differences still confound me.

---

Can someone wiser than I am please help me get this code more maintainable? The two blatant code repetitions I mentioned are my main concerns, but I'm open to all criticisms large and small. I'm only a few months old in the worlds of C#, .NET, and Windows development.

Answer 1

You actually don't need to do this:

this.SomeControl.SomeEvent += new SomeEventHandler(SomeDelegate);

You can simply do this:

this.SomeControl.SomeEvent += SomeDelegate;

Armed with this knowledge you can create that method you were craving:

private static void RegisterFileDropHandler(ButtonEdit control, DragEventHandler handler)
{
    control.AllowDrop = true;
    control.DragEnter -= ActivateIfSingleFileDrag;
    control.DragEnter += ActivateIfSingleFileDrag;
    control.DragDrop -= handler;
    control.DragDrop += handler;
}

I'll come back to your other point later if I have time :)

Update

As your second bit of replication:

Create another method:

private static void DoSomethingWithFile(ButtonEdit sender, DragEventArgs evt, string errorMessage)
{
    string[] files = (string[])evt.Data.GetData(DataFormats.FileDrop);
    if (!files.Any())
    {
        return;
    }
    try
    {
        // Figure out which button raised the event - something like:
        // sender.Id
        // Then execute a method specific to that button.
    } 
    catch (ArgumentException ex)
    {
        DisplayAlert(string.Format("{0}\n{1}", errorMessage ,ex.Message);
    }
}

Now all of your handlers just call that method:

private void TryExtractCalibration(object sender, DragEventArgs evt)
{
    DoSomethingWithFile((ButtonEdit)sender, evt, "Could not load Calibration from file!");
}

Answer 2

// Using -= then += on a handler ensures only one is ever added.  

This is basically true, but in your szenario superfluous, because the SetDragDropHandlers() method is only called from the Load event of the form. You just can skip the removing of the handler.

---

In addition to @RobH's answer:

If you are using C# 6 you could use some syntax sugar by using string interpolation by replacing this

catch (ArgumentException ex)
{
    DisplayAlert(string.Format("{0}\n{1}", errorMessage ,ex.Message);
}  

with

catch (ArgumentException ex)
{
    DisplayAlert($"{errorMessage}\n{ex.Message}");
}

taking into account @RobH comment we should change it to

catch (ArgumentException ex)
{
    DisplayAlert($"{errorMessage}{Environment.NewLine}{ex.Message}");
}