2
\$\begingroup\$

I have a comboBox and a TextBox in the View and when the view is loaded, the ComboBox should be populated. After that when a user selects particular item from the ComboBox, its related ValueMember should be displayed in the TextBox.

The View doesn't know about DAL or Presenter and, it only has properties and events.

VIEW

public partial class frmBankAccount : Form, IBankAccountView
{

    public event EventHandler OnLoadEmployees;

    public BindingSource LoadingEmployees
    {
        set { cmbName.DataSource = value; cmbName.DisplayMember = "Name"; cmbName.ValueMember = "Emp_ID"; }
    } 

    private void frmBankAccount_Load(object sender, EventArgs e)
    {
         OnLoadEmployees(sender, e);
    }

    private void cmbName_SelectedValueChanged(object sender, EventArgs e)
    {
        txtEID.Text = cmbName.SelectedValue.ToString();
    }

}

When the event fires, Presenter gets a DataTable from DataService and binds it to the ComboBox through the property.

PRESENTER

class BankAccountPresenter : BasePresenter
{
    public BankAccountPresenter(IBankAccount model, IBankAccountView view, IDataService dataService )  
    {
        _DataService = dataService;
        _Model = model;
        _View = view;

        this.WireUpViewEvents(); 
    }

    private void WireUpViewEvents() 
    {
        _View.OnLoadEmployees += new EventHandler(_View_OnLoadEmployees);
    }

    private void _View_OnLoadEmployees(object sender, EventArgs e)
    {
        LoadEmployeeNames();
    }

    private void LoadEmployeeNames()
    {
        BindingSource bs = new BindingSource();
        bs.DataSource = _DataService.LoadEmplyees(); //DataService returns a DataTabel with "Emp_ID" and "Name"
        _View.LoadingEmployees = bs;    
    }

    //Main window will call this method to load the View
    public void Show()
    {
        _View.ShowDialog();
    }

}

NOTE: When populating ComboBox since I need only two fields (Emp_ID and Name), The DataService returns a DataTable instead of the Model as the model has about 10 other fields as well. The Model will be used in subsequent tasks like Search, Insert, Update etc.

Could you please review this code and provide your feedback? You may give your attention to the way that I've used properties to set DisplayMember and ValueMember of the Combo. If there is anything which is not acceptable kindly let me know with an alternate solution.

\$\endgroup\$
  • 1
    \$\begingroup\$ Unless you are supporting some 10 years old project, you should consider switching to wpf. Winforms is an obsolete framework. \$\endgroup\$ – Nikita B Jun 30 '14 at 10:56
1
\$\begingroup\$

It looks pretty clean. As far as I can tell, you have a good separation of concerns. Everything is properly cased and your functions have verb-noun names. All good things.

The only nitpick I have is the bs variable. I'm not a fan of ultra short names of any kind. You could call it source or some such thing. Seriously though, that's a huge nitpick. I just couldn't find anything else to comment on.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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