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

Presenter creating Views in c# MVP

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

Problem

I have a c# WinForms application in which I have tried to implement the Model View Presenter pattern with Passive View. I create new Views in the Presenter of my 'home screen'. I was wondering if this is bad practice, and if it is, what an alternative might be to resolve this. Also, general improvements are more than welcome.

I have asked a question before about my MVP implementation here (you can check it out if you want more of an idea about how I have implemented the pattern in other parts of my application.

You can find my code below.

View

```
public partial class HomeScreenView : Form, IHomeScreenView
{
private HomeScreenPresenter _presenter;

// Public members.
public List ExistingAssessments
{
get
{
var assessments = new List();
for(int i = 0; i < recentAssessments.Lines.Count(); i++)
{
assessments.Add(recentAssessments.Lines[i]);
}
return assessments;
}
set
{
recentAssessments.Text = "";
foreach (var assessment in value)
{
recentAssessments.Text = assessment + Environment.NewLine + recentAssessments.Text;
}
}
}

// Public events.
public event EventHandler InitializingAssessments;
public event EventHandler CreatingNewAssessment;
public event EventHandler AddingNewStandard;
public event EventHandler OpeningAssessment;
public event EventHandler GeneratingReport;

// Initialize homescreen. Create an instance of the presenter with a reference the view itsself.
public HomeScreenView(IAssessmentsModel model)
{
InitializeComponent();
_presenter = new HomeScreenPresenter(this, model);
}

// Fires the initializing assessments event.
private void HomeScreenView_Activated(object sender, EventArgs e)
{
InitializingAssessments?.Invoke(this, EventArgs.Empty);
}

// Fires the crea

Solution

imo, you can simplify this bit

var assessments = _model.GetDataList("Assessments", "assessment_name");

if (assessments.Any())
{
  HashSet items = new HashSet(assessments);
  assessments = items.ToList();

  for (int i = 0; i < assessments.Count; i++)
  {
      assessments[i] = (assessments[i] + ": " + _model.CheckStatus(assessments[i]));
  }

  _view.ExistingAssessments = assessments;
}


to this

var assessments = _model.GetDataList("Assessments", "assessment_name").Distinct();
if (assessments.Any())
{
    //assessments.ForEach(a => a += $":{_model.CheckStatus(a)}"); -> wont update values
    assessments = assessments.Select(a => a += $":{_model.CheckStatus(a)}").ToList();
    _view.ExistingAssessments = assessments;
}

Code Snippets

var assessments = _model.GetDataList("Assessments", "assessment_name");

if (assessments.Any())
{
  HashSet<string> items = new HashSet<string>(assessments);
  assessments = items.ToList();

  for (int i = 0; i < assessments.Count; i++)
  {
      assessments[i] = (assessments[i] + ": " + _model.CheckStatus(assessments[i]));
  }

  _view.ExistingAssessments = assessments;
}
var assessments = _model.GetDataList("Assessments", "assessment_name").Distinct();
if (assessments.Any())
{
    //assessments.ForEach(a => a += $":{_model.CheckStatus(a)}"); -> wont update values
    assessments = assessments.Select(a => a += $":{_model.CheckStatus(a)}").ToList();
    _view.ExistingAssessments = assessments;
}

Context

StackExchange Code Review Q#147425, answer score: 2

Revisions (0)

No revisions yet.