Controls Edits of DataGridview in Winform

I want to refactor this code that is currently in a partial Form Class and deals with edits made on certain cells in a datagridview (in this case called dgvReplenish).

I would like to refactor so that I can create unit tests to be able to test the logic that effects changing the back color of cells, and updating other cells.

The problem I am having is how to refactor logic to an external class, that has references to the winform controls.

Form

All following code is part of public partial class ReplenForm : Form

DataGridView Event Handlers

private string lastEditedCellValue;

    private void dgvReplenish_CellBeginEdit(object sender, DataGridViewCellCancelEventArgs e)
    {
        lastEditedCellValue = dgvReplenish.Rows[e.RowIndex].Cells[e.ColumnIndex].Value.ToString();
    }

    private void dgvReplenish_CellValidating(Object sender, DataGridViewCellValidatingEventArgs e)
    {
        if (e.ColumnIndex == (int)ProductColumnIndex.ReplenishAmount)
        {
            ValidateEditOfReplenishAmount(e);
        }
    }

    private void dgvReplenish_CellClicked(object sender, DataGridViewCellEventArgs e)
    {
        PopulateSelectedLineDetails(e.RowIndex);
    }

    private void dgvReplenish_CellFormatting(object sender, DataGridViewCellFormattingEventArgs e)
    {
        ChangeColorOfWarehouseShopName(e);
    }

    private void dgvReplenish_CellValueChanged(object sender, DataGridViewCellEventArgs e)
    {
        if (e.ColumnIndex == (int)ProductColumnIndex.ReplenishAmount){

            OnReplenishAmountEdit(e.RowIndex, e.ColumnIndex);
            AddEditedRowToEditedRowsDictionary(e.RowIndex, e.ColumnIndex);
        }

        ChangeAvailableStockOnEditOfReplenishAmount(e.RowIndex, e.ColumnIndex);
    }

Cell Edit functions

```
///
/// When Replenish Amount is edited change back colour of cell and update database
///
/// DataGridView Cell Arguments object
private void OnReplenishAmountEdit(int rowIndex, int columnIndex)

The truth is, it's really very hard to refactor this code out of the code behind once it's been written like this. I would suggest that you start writing your tests first, for now at least. The benefit there is that you have to design your code to be testable to begin with. If you decide later (once you have an idea of how to better design your code), to test after the fact, that's fine. The important thing right now is to separate the concerns.

You need some Model-View-Presenter in your life. It's more or less the default design pattern for WinForms. There are two flavors, passive and supervising controller.

The very first thing you need to do is make sure that your form is not responsible for retrieving the DataTable. You will never be able to test this code if the form is responsible for retrieving the data. It needs to be provided the data via a constructor or property. The Presenter should be responsible for providing it to the View. Most likely, it will delegate this responsibility to another class, but ultimately, the Presenter will pass the information from that class to the view.

You'll also need to properly model the data. Right now, there's no concept of a Part in this code, but there's obviously a business object that is very much a Part and your form displays a list of Parts. I'd start with defining a simple class defining the data structure itself, and add logic to it as necessary. Eventually, this is where your validation logic will lie.

internal class Part
{
    int Id { get; private set; } //consider wrapping this in a simple PartId wrapper around int.
    string Description { get; private set; }
    byte Rsp { get; private set; }
    string Identifier { get; private set; }

    // lots more properties

    internal Part(/* one param for each property */)
    {
      //...
    }
}

And now, to define an interface for your view. (Note that I left everything but what you need for the data grid out.)

internal interface IPartsView
{
    IList Parts { get; set; }
}

So, your Form's signature will need to change a bit to implement this.

public partial class ReplenForm : Form, IPartsView

Then in the Getter/Setter, you'll transform the list into something bindable (personally, I like the BindingList) and bind it to the datagrid. There's a great example of binding a list to a datatable and datagridview on dotnetpearls.

So, we've gotten all of the from outside of the form and injected it in. Great, but what about all those pesky events? Well, we'll add some to the interface for the presenter to respond to. The Form will just delegate it's own events by raising IPartsView's events.

For example, let's handle a selected record change.

internal interface IPartsView
{
    IList Parts { get; set; }

    event EventHandler SelectedItemChanged;
}

Back in your form you raise the interface's event whenever the DataGridView's event is raised.

public EventHandler SelectedItemChanged;
private void dgvReplenish_CellClicked(object sender, DataGridViewCellEventArgs e)
{
    var handler = SelectedItemChanged;
    if (handler != null)
    {
        SelectedItemChanged(this, e);
    } 
}

Which moves PopulateSelectedLineDetails into your presenter. It catches the raised event, and interacts with the View's interface, performing this work.

This means, that you can now relatively simply test your code. I like using Moq for this. Moq makes it easy to mock out the view and test the presenter. Because that's what we're really aiming for here. Your Form (IPartsView) needs to be dead dumb simple. It shouldn't need to be tested. If the presenter and model do all of the work, then there's no need to test the view.

A simple test could look something like this.

[TestMethod]
public void FieldXIsUpdatedAfterSelectionChange()
{
    //arrange

    var parts = //some code to create a dummy parts list

    var view = new Mock();
    view.SetupAllProperties();

    var presenter = new PartsPresenter(view.Object, parts);

    //act 

    //simulate selection change
    view.Raise(v => v.SelectedItemChanged += null, new CustomEventArgs(2)); // row 2

    //assert
    Assert.AreEqual(expected, view.SomeProperty);
}

I know that was a lot to take in, and not a whole lot of actual code from your project, but you really do have yourself in a pickle here. I recommend implementing something simple with this pattern before trying to refactor to it with this monolithic code behind that you have there. I fear that if you try without having a simpler experience first, that you'll fail and never learn how powerfully awesome this pattern can be in your code.

Good luck!

StackExchange Code Review Q#92748, answer score: 5