Binding a DataGridView to a List

Question

I'm new to Databinding in MVP and I want to bind my DataGridView to a object list. This grid may be populated in two ways.

There are TextBoxes in the form and users can enter text, and when they press Add button, a new object is instantiated with the provided fields and then it is added to the list.

Other way is, when a user searches for objects, matching objects are loaded from database to the list. Also after searching users should be able to add new items to this list as in the first method.

Finally this list is binded to the DataGridView and details are shown in the grid. Now the user could do any changes to the items shown in the grid and if it's all right, the user could save them.

So in my presenter I have three methods for the above requirements.

    Class AttendancePresenter : BasePresenter
    {
        private IAttendance _Model;
        private readonly IAttendanceView _View;
        private readonly IDataService _DataService;
        private readonly IMessageService _MessageService;
        List<IAttendance> AttendanceList = new List<IAttendance>();

        public AttendancePresenter( IAttendance model, IAttendanceView view, IDataService dataService, IMessageService messageService ) : base(messageService)
        {
            _Model = model;
            _View = view;
            _DataService = dataService;
            _MessageService = messageService;

            SetModelPropertiesFromView(_Model, _View);
            WireUpViewEvents();
        }

        //Add new attendance to the list and the the list is binded to the grid
        private void _View_OnAddAttendance(object sender, EventArgs e)
        {

            SetModelPropertiesFromView(_Model, _View); // call base class method to update the Mode with uptodate data from the view
            IAttendance attendanceModel = new Attendance(_Model); // Use Model's copy constructor to get a clone
            AttendanceList.Add(attendanceModel);
            var bindingList = new BindingList<IAttendance>(AttendanceList);
            var source = new BindingSource(bindingList, null);
            _View.AttendanceGrid = source;
        }

        //Get attendance from the database and show on the grid
        void _View_OnSearchForAttendance(object sender, EventArgs e)
        {
            AttendanceList = _DataService.GetAttendance(_View.InTime, _View.PointID, AttendanceList);
            var bindingList = new System.ComponentModel.BindingList<IAttendance>(AttendanceList);
            var source = new BindingSource(bindingList, null);
            _View.AttendanceGrid = source;
        }

        //Pass attendance list to DAL to insert into the database
        private void _View_OnSaveAttendance(object sender, EventArgs e)
        {
             var result = _DataService.InsertAttendance(AttendanceList);
             AttendanceList.Clear();

            _MessageService.ShowResult(result);
        }

    }

I'm not sure the way I've binded things is right. And also I wonder if I could use a single binding list here as I'm binding same list to the same DataGridView! Please review my code and give your advice.

Answer 1

First some nitpicks:

• Variables should be camelCased, not PascalCased. The underscore prefixes are okay, but _Model is not. Let the underscore be your visual indicator. What you've done is confusing on the eyes.

• Don't fully qualify names unless you have to.

  var bindingList = new System.ComponentModel.BindingList<IAttendance>(AttendanceList);
  

  Else where you directly use BindingList, so System.ComponentModel must be in your usings somewhere. Using the fully qualified call clutters the code.

• You could extract this duplicated code into its own method.

  var bindingList = new BindingList<IAttendance>(AttendanceList);
  var source = new BindingSource(bindingList, null);
  _View.AttendanceGrid = source;
  

I don't see anything particularly bad with what you've done, but perhaps I'm inexperienced or lacking larger context. The only thing I might recommend, would be to keep a bindingList around and just call the clear method instead of creating a new instance from scratch all the time. Let me be clear though, I'm not 100% sure that's good advice or any better than "newing it up".