HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Binding Lists of Commits to a DataGridView in Winforms with an MVP architecture

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
architecturewithmvpdatagridviewwinformscommitsbindinglists

Problem

I'm using Winforms and I find this pattern showing up in my code a lot. It works, but I'm unsure if it's best. The goal is to pass an IList of items through a view interface and then bind that list to a DataGridView in the concrete GUI class. I've been accomplishing this by keeping a private BindingList in the control, and converting the IList when getting/setting the properties. I can't help but feel that there's a better way to do this databinding.

The GUI looks like this.

The View interface.

public interface IUnSyncedCommitsView
{
    event EventHandler Fetch;
    event EventHandler Pull;
    event EventHandler Push;
    event EventHandler Sync;

    IList IncomingCommits { get; set; }
    IList OutgoingCommits { get; set; } 
}


The UserControl code behind.

```
[ExcludeFromCodeCoverage]
public partial class UnSyncedCommitsControl : UserControl, IUnSyncedCommitsView
{
public UnSyncedCommitsControl()
{
InitializeComponent();

SetText();
}

private void SetText()
{
CurrentBranchLabel.Text = RubberduckUI.SourceControl_CurrentBranchLabel;
FetchIncomingCommitsButton.Text = RubberduckUI.SourceControl_FetchCommitsLabel;
PullButton.Text = RubberduckUI.SourceControl_PullCommitsLabel;
PushButton.Text = RubberduckUI.SourceControl_PushCommitsLabel;
SyncButton.Text = RubberduckUI.SourceControl_SyncCommitsLabel;

IncomingCommitsBox.Text = RubberduckUI.SourceControl_IncomingCommits;
OutgoingCommitsBox.Text = RubberduckUI.SourceControl_OutgoingCommits;
}

private BindingList _incomingCommits;
public IList IncomingCommits
{
get { return _incomingCommits; }
set
{
_incomingCommits = new BindingList(value);
this.IncomingCommitsGrid.DataSource = _incomingCommits;
}
}

private BindingList _outgoingCommits;
public IList OutgoingCommits
{
get { return _outgoingCommits; }
set

Solution

A problem I see here is that the current approach will generate deeply nested BindingList objects if you repeatedly call the setter with a list you previously got from the getter. If you see what I mean. Probably you don't really do such thing in practice, but it's still ugly.

I'm going to assume that you generally want the getter return the same list that was originally passed to the setter. That is, if you passed some list x to the setter, you want that list x back from the getter, not some wrapped(x) list.

To make this work, you would need to keep the original list, or have a way to access it again if the setter puts it in a wrapper.
It seems this can be done using a BindingSource:

  • Create a BindingSource with a BindingList and null parameter, and keep it in a field



  • Bind your grid to this BindingSource



  • Make the list getter return the underlying list with bindingSource.List



  • Make the list setter replace the underlying list, either by replacing the list in the BindingList, or by replacing the BindingList in the BindingSource



This last item is a work in progress.
I spent some time searching through the docs for it,
but it seems harder than I expected.
(Btw I found this discussion illuminating.)

If all else fails, you could clear the underlying list and add all elements from the incoming list.
Depending on how you use the getters and setters,
you might want to do some defensive copies as appropriate.

Unrelated to your main concern, the code duplication when calling the fetch/push/... handlers is not great, with the boilerplate null checks. It would be good to create a helper method CallHandlerIfNotNull or similar.

Context

StackExchange Code Review Q#94885, answer score: 4

Revisions (0)

No revisions yet.