Problem
I have a c# WinForms application in which I have tried to implement the Model View Presenter pattern with Passive View. The View derives from a parent IView which propagates the public members and events of the View to its Presenter. In this way, the Presenter can utilize these field and react to the events without relying on the implementation of the View (forms library and such). The View only fires events and performs input validation logic.
I have included one of my Views below. I was wondering if this View is ‘Passive’ enough, so that my program is compliant with the pattern and if my general idea of the pattern is correct. Any other improvements are welcome as well. This is my first ever application, c# experiece, and implementation of the MVP pattern so bear with me.
View
public partial class StandardScreenView : Form, IStandardScreenView
{
// Public members.
public List<string> QuestionStandards
{
get { return currentQuestionStandards.Items.Cast<string>().ToList(); }
set
{
currentQuestionStandards.DataSource = value;
currentQuestionStandards.SelectedItem = null;
}
}
public List<string> MaturityStandards
{
get { return currentMaturityStandards.Items.Cast<string>().ToList(); }
set
{
currentMaturityStandards.DataSource = value;
currentMaturityStandards.SelectedItem = null;
}
}
public List<string> ComplianceStandards
{
get { return currentComplianceStandards.Items.Cast<string>().ToList(); }
set
{
currentComplianceStandards.DataSource = value;
currentComplianceStandards.SelectedItem = null;
}
}
public string SelectedQuestionStandard
{
get
{
if(currentQuestionStandards != null)
{
return currentQuestionStandards.SelectedItem.ToString();
}
return null;
}
set { currentQuestionStandards.SelectedItem = value; }
}
public string SelectedMaturityStandard
{
get
{
if(currentMaturityStandards != null)
{
return currentMaturityStandards.SelectedItem.ToString();
}
return null;
}
set { currentMaturityStandards.SelectedItem = value; }
}
public string SelectedComplianceStandard
{
get
{
if(currentComplianceStandards != null)
{
return currentComplianceStandards.SelectedItem.ToString();
}
return null;
}
set { currentComplianceStandards.SelectedItem = value; }
}
// Public events.
public event EventHandler<EventArgs> InitializingStandards;
public event EventHandler<string> AddingQuestionStandard;
public event EventHandler<string> AddingMaturityStandard;
public event EventHandler<string> AddingComplianceStandard;
public event EventHandler<EventArgs> DeletingQuestionStandard;
public event EventHandler<EventArgs> DeletingMaturityStandard;
public event EventHandler<EventArgs> DeletingComplianceStandard;
// Initialize standardscreen. Create an instance of the presenter with a reference the view itsself.
public StandardScreenView(IStandardsModel model)
{
InitializeComponent();
var standardScreenPresenter = new StandardScreenPresenter(this, model);
Show();
this.ActiveControl = questionStandardLabel;
}
// Initializes the standards.
private void StandardScreenView_Activated(object sender, EventArgs e)
{
InitializingStandards?.Invoke(this, EventArgs.Empty);
}
// Gets the filename and fires the question standard adding event.
private void newQuestionStandardButton_Click(object sender, EventArgs e)
{
Cursor.Current = Cursors.WaitCursor;
string fileName = GetFileName();
if (fileName != null)
{
AddingQuestionStandard?.Invoke(this, fileName);
}
}
// Gets the filename and fires the maturity standard adding event.
private void newMaturityStandardButton_Click(object sender, EventArgs e)
{
Cursor.Current = Cursors.WaitCursor;
string fileName = GetFileName();
if (fileName != null)
{
AddingMaturityStandard?.Invoke(this, fileName);
}
}
// Gets the filename and fires the compliance standard adding event.
private void newComplianceStandardButton_Click(object sender, EventArgs e)
{
Cursor.Current = Cursors.WaitCursor;
string fileName = GetFileName();
if (fileName != null)
{
AddingComplianceStandard?.Invoke(this, fileName);
}
}
// Opens the openfile dialog and checks the result. If the result is OK, the form is closed and the filename is returned.
public string GetFileName()
{
DialogResult result = openFileDialog1.ShowDialog();
string fileName = openFileDialog1.FileName;
if (result == DialogResult.OK && (Path.GetExtension(openFileDialog1.FileName) == ".txt"))
{
return fileName;
}
else if (result == DialogResult.Cancel)
{
return null;
}
else
{
Message("You have selected a file with an illegal extension. Please try again and select a *.txt file.");
return null;
}
}
// Fires the deleting question standards event.
private void deleteQuestionStandardButton_Click(object sender, EventArgs e)
{
string temp = SelectedQuestionStandard;
if (temp != null && MessageBox.Show("Are you sure you want to delete this standard from the database?", ""
, MessageBoxButtons.YesNo) == DialogResult.Yes)
{
SelectedQuestionStandard = temp;
DeletingQuestionStandard?.Invoke(this, EventArgs.Empty);
}
}
// Fires the deleting maturity standards event.
private void deleteMaturityStandardButton_Click(object sender, EventArgs e)
{
string temp = SelectedMaturityStandard;
if (temp != null && MessageBox.Show("Are you sure you want to delete this standard from the database?", ""
, MessageBoxButtons.YesNo) == DialogResult.Yes)
{
SelectedMaturityStandard = temp;
DeletingMaturityStandard?.Invoke(this, EventArgs.Empty);
}
}
// Fires the deleting compliance standards event.
private void deleteComplianceStandardButton_Click(object sender, EventArgs e)
{
string temp = SelectedComplianceStandard;
if (temp != null && MessageBox.Show("Are you sure you want to delete this standard from the database?", ""
, MessageBoxButtons.YesNo) == DialogResult.Yes)
{
SelectedComplianceStandard = temp;
DeletingComplianceStandard?.Invoke(this, EventArgs.Empty);
}
}
// Displays a message.
public void Message(string message)
{
MessageBox.Show(message);
}
// Closes the view.
public void CloseView()
{
Close();
}
}
IView
public interface IStandardScreenView
{
// Public members.
List<string> QuestionStandards { get; set; }
List<string> MaturityStandards { get; set; }
List<string> ComplianceStandards { get; set; }
string SelectedQuestionStandard { get; set; }
string SelectedMaturityStandard { get; set; }
string SelectedComplianceStandard { get; set; }
// Public events.
event EventHandler<EventArgs> InitializingStandards;
event EventHandler<string> AddingQuestionStandard;
event EventHandler<string> AddingMaturityStandard;
event EventHandler<string> AddingComplianceStandard;
event EventHandler<EventArgs> DeletingQuestionStandard;
event EventHandler<EventArgs> DeletingMaturityStandard;
event EventHandler<EventArgs> DeletingComplianceStandard;
// Public functions.
void Message(string message);
void CloseView();
}
Presenter
public class StandardScreenPresenter
{
// Private view and model.
private readonly IStandardScreenView _view;
private IStandardsModel _model;
// Initialize the standard screen presenter with an IStandardScreenView and a new assessments model. Subsribe to necessary events.
public StandardScreenPresenter(IStandardScreenView view, IStandardsModel model)
{
_view = view;
_model = model;
_view.InitializingStandards += InitializeStandards;
_view.AddingQuestionStandard += AddQuestionStandard;
_view.AddingMaturityStandard += AddMaturityStandard;
_view.AddingComplianceStandard += AddComplianceStandard;
_view.DeletingQuestionStandard += DeleteQuestionStandard;
_view.DeletingMaturityStandard += DeleteMaturityStandard;
_view.DeletingComplianceStandard += DeleteComplianceStandard;
}
// Initializes the standards.
public void InitializeStandards(object sender, EventArgs e)
{
_view.QuestionStandards = null;
_view.MaturityStandards = null;
_view.ComplianceStandards = null;
try
{
var questionStandards = _model.GetStandards("Questions", "question_standard");
var maturityStandards = _model.GetStandards("MaturityAnswers", "m_answer_standard");
var complianceStandards = _model.GetStandards("ComplianceAnswers", "c_answer_standard");
if (questionStandards != null && questionStandards.Any())
{
HashSet<string> items = new HashSet<string>(questionStandards);
questionStandards = items.ToList();
_view.QuestionStandards = questionStandards;
}
if (maturityStandards != null && maturityStandards.Any())
{
HashSet<string> items = new HashSet<string>(maturityStandards);
maturityStandards = items.ToList();
_view.MaturityStandards = maturityStandards;
}
if (complianceStandards != null && complianceStandards.Any())
{
HashSet<string> items = new HashSet<string>(complianceStandards);
complianceStandards = items.ToList();
_view.ComplianceStandards = complianceStandards;
}
}
catch(Exception ex)
{
_view.Message("Error - initializing standards has failed.n" +
"Error message - " + ex.Message);
}
}
// Adds the new question standard file.
public void AddQuestionStandard(object sender, string standardName)
{
try
{
_model.AddNewStandard("Questions", "question_id, question_value, question_standard", standardName);
_view.Message("The new question standard has been added to the database.");
}
catch(Exception ex)
{
_view.Message("Error - Adding question standard to database has failed, please make sure the question id's are unique and the filepath is valid."
+ Environment.NewLine + "Error message - " + ex.Message);
}
}
// Adds the new maturity standard file.
public void AddMaturityStandard(object sender, string standardName)
{
try
{
_model.AddNewStandard("MaturityAnswers", "m_answer_id, m_answer_value, m_answer_standard", standardName);
_view.Message("The new maturity standard has been added to the database.");
}
catch (Exception ex)
{
_view.Message("Error - Adding maturity standard to database has failed, please make sure the answer id's are unique and the filepath is valid."
+ Environment.NewLine + "Error message - " + ex.Message);
}
}
// Adds the new compliance standard file.
public void AddComplianceStandard(object sender, string standardName)
{
try
{
_model.AddNewStandard("ComplianceAnswers", "c_answer_id, c_answer_value, c_answer_standard", standardName);
_view.Message("The new compliance standard has been added to the database.");
}
catch (Exception ex)
{
_view.Message("Error - Adding compliance standard to database has failed, please make sure the asnwer id's are unique and the filepath is valid."
+ Environment.NewLine + "Error message - " + ex.Message);
}
}
// Deletes the question standard.
public void DeleteQuestionStandard(object sender, EventArgs e)
{
try
{
_model.DeleteStandard("Questions", "question_standard", _view.SelectedQuestionStandard);
_view.Message("The standard has been deleted.");
}
catch(Exception ex)
{
_view.Message("Error - Deleting standard has failed.n" +
"Error message - " + ex.Message);
}
}
// Deletes the maturity standard.
public void DeleteMaturityStandard(object sender, EventArgs e)
{
try
{
_model.DeleteStandard("MaturityAnswers", "m_answer_standard", _view.SelectedMaturityStandard);
_view.Message("The standard has been deleted.");
}
catch (Exception ex)
{
_view.Message("Error - Deleting standard has failed.n" +
"Error message - " + ex.Message);
}
}
// Deletes the compliance standard.
public void DeleteComplianceStandard(object sender, EventArgs e)
{
try
{
_model.DeleteStandard("ComplianceAnswers", "c_answer_standard", _view.SelectedComplianceStandard);
_view.Message("The standard has been deleted.");
}
catch (Exception ex)
{
_view.Message("Error - Deleting standard has failed.n" +
"Error message - " + ex.Message);
}
}
}
Solution
event EventHandler<EventArgs> InitializingStandards;
event EventHandler<string> AddingQuestionStandard;
This is not exactly how we implement events.
The EventHandler
Delegate already works with EventArgs
so it’s redundant to use the generic one:
public delegate void EventHandler(object sender, EventArgs e)
it’s the same as if you wrote:
event EventHandler InitializingStandards;
but if you use the generic overload then the argument should be derived from EventArgs
:
public delegate void EventHandler<TEventArgs>(object sender, TEventArgs e)
The standard signature of an event handler delegate defines a method that does not return a value. This method’s first parameter is of type Object and refers to the instance that raises the event. Its second parameter is derived from type EventArgs and holds the event data. If the event does not generate event data, the second parameter is simply the value of the EventArgs.Empty field. Otherwise, the second parameter is a type derived from EventArgs and supplies any fields or properties needed to hold the event data.
(Emphasis mine)
For some strange reason the delegate does not enforce that and has no where TEventArgs : EventArgs
constraint. But if you want to stick to the convention then you should create new types like:
class AddingQuestionStandardEventArgs : EventArgs
{
AddingQuestionStandardEventArgs(string standardName)
{
StandardName = standardName;
}
public string StandardName { get; }
}
to use them instead of the string:
event EventHandler<AddingQuestionStandardEventArgs> AddingQuestionStandard;