Design issue in a MVP win forms application

Question

In a module of a win forms application I have a hierarchy of classes like BaseRecovery, LoanRecovery and FineRecovery. Both LoanRecovery and FineRecovery inherits from BaseRecovery. All these models have one view called RecoveryForm where you can enter/view loans and fines of employees. and I'm planing to use single Presenter class called RecoveryPresenter which inherits from BasePresenter. We have a DataService Class for db transactions

LoanRecovery class looks like this...

Class LoanRecovery : BaseRecovery
{
    public string LoanID {get;set;}
    public DateTime IssueDate {get;set;}
    public Decimal Amount {get;set;}
    .
    .
    .
}

So I'd do the following in Programe.cs.

IBaseRcovery recovery=null;
IRecoveryForm recoveryForm = new RecoveryForm();
IDataService dataService = new DataService();

BasePresenter presenter = new RecoveryPresenter( recovery, recoveryForm, dataService );
presenter.Show(); // This will display the form recoveryForm

In the presenter I would say

public RecoveryPresenter( IBaseRecover model, IRecoveryForm view, IDataService dataService )
{
    this._Model = model;
    this._View = view;
    this._DataService = dataService;            
    WireUpViewEvents();

}

Now lets say if I need to give a loan, I would run SetModelPropertisFromView() mehtod in the BasePresenter class which use reflection. But before that I should pass an instance of LoanRecovery class (i.e. _Model) to that method together with _View. Like wise we can do same thing for all children classes as follows...

    public void Issue()
    {
        if (_View.Type == "Loan")
        {
            _Model = new LoanRecovery();
            SetModelPropertiesFromView(_Model, _View, _DataService);
            _dataService.InsertLoan(_Model); //Error
        }

        if (_View.Type == "Fine")
        {
            _Model = new FineRecovery();
            SetModelPropertiesFromView(_Model, _View, _DataService);
            _DataService.InsertFine(_Model); //Error
        }

        if (_View.Type == "Insurance")
        {
            _Model = new InsuranceRecovery();
            SetModelPropertiesFromView(_Model, _View, _DataService);
            _DataService.InsertFine(_Model); //Error
        }


    } 

Everything fine until the last line of the above if blocks. The issue is data access method in the DataService class required a child instance not a base class instance.

  public void InsertLoan( LoanRecovery loan)
    {
        using (SqlConnection sqlConnection = new SqlConnection(db.GetConnectionString))
        {
            SqlCommand sqlCommand = new SqlCommand("BEGIN INSERT INTO recovery ;
            sqlCommand.Parameters.Add("@empID", SqlDbType.Char).Value = loan.EmployeeID;
            sqlCommand.Parameters.Add("@loanType", SqlDbType.VarChar).Value = loan.Type;
            sqlCommand.Parameters.Add("@loanAmount", SqlDbType.Decimal).Value = loan.FullAmount;
            sqlCommand.Parameters.Add("@loanDuration", SqlDbType.Int).Value = loan.Duration;
            sqlConnection.Open();
            sqlCommand.ExecuteNonQuery();
        }
    }

So I solved the problem as follows...

    public void Issue()
    {
        if (_View.Type == "Loan")
        {
            LoanRecovery loanModel = new LoanRecovery(); //Creates a child instance
            SetModelPropertiesFromView(loanModel, _View, _DataService);
            _DataService.InsertLoan(loanModel);
        }
    }

Now its working but my worry is that here I'm not using the injected instance through the constructor rather newing a child object which creates a dependency. Can anybody propose a better design to solve this issue please?

Answer 1

OK, if I understand correctly, your IDataService interface has specific methods for saving your concrete classes that implement IBaseRecover, but to call the correct save method, you need to know the concrete type of IBaseRecover. Why not push the save responsibility into the concrete implementations of IBaseRecover because they know best which method should be called. You could do this like follows:

interface IBaseRecover{
    void Save();
    ....
}


class LoanRecovery : IBaseRecover {
    private IDataService _dataService;
    LoadRecovery(IDataService dataService){
        _dataService = dataService;
    }}

    void Save(){
       _dataService.InsertLoan(this);
    }
}

OK, here's another thought based on your comment. I think the complexity here comes from the fact that you are using a view that can represent three different types of models, and even though they all derive from the same base class, your view has inputs that are specific to each implementation. That's fine, but I think you'd benefit from a different variation of the MVP pattern. Martin Fowler has since broken up the MVP pattern into Supervising Contoller and Passive View. I think Passive View would serve you well here. In that case, your view would expose the values of it's input elements as properties so that the Presenter could get Recovery properties such as IssueDate and Amount without have to know about the view components. So the code for the Insert Action on the view might look something like the following:

public decimal Amount{
    get{
        return Convert.ToDecimal(txtAmount.text);
    }
}

void btnInsert_Click(Object sender, EventArgs e){
    presenter.Insert();
}

Note that it's ok for your view to have an instance of the Presenter. Now the Presenter Insert method might look something like this:

public void Insert(){
    if(_view.Type == "Loan"){
       var model = new LoadRecovery();
       model.IssueDate = _view.IssueDate;
       model.Amount = _view.Amount;
       _dataService.InsertLoan(model);
    }
}

And if you were also first displaying a Recovery to be updated instead of inserted:

public void Show(){
    _view.IssueDate = model.IssueDate;
    _view.Amount = model.Amount;
}