Separation of concerns
public List<string> ExistingAssessments { get { var assessments = new List<string>(); 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; } } }
This is an interesing property that desperately needs improvement.
It stores the data lines as as string that is apparently being splitted internally and you get back the lines via the Lines
property. A lot of work to just store a few lines of text.
You should drop the string concatenation here which is with the +
operator - that is slow by the way - and store the lines raw in a list. It's the job of the Text
or some other property/method to build a full/formatted-string.
This property should ideally do only this:
public IEnumerable<string> ExistingAssessments
{
get
{
return recentAssessments.Lines.ToList();
}
set
{
recentAssessments.Lines = value.ToList();
}
}
You can ToList
the return value and the new value so that you cannot by accident modify it from outside.
Everything else is beyond its responsibility scope.
Other suggestions
var assessments = new List<string>(); for (int i = 0; i < recentAssessments.Lines.Count(); i++) { assessments.Add(recentAssessments.Lines[i]); } return assessments;
This is much less code when done with LINQ:
return recentAssessments.Lines.ToList();
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; } else _view.ExistingAssessments = new List<string>();
There's no need for the HashSet
, for the Any
and the else
part. Just create the new list right away and use Distinct
. If there are no assessments
it will remain simply empty. With C# 6 it's really short:
var assessments =
_model.GetDataList("Assessments", "assessment_name")
.Distinct()
.Select(x => $"{x}: {_model.CheckStatus(x)}")
.ToList();
_view.ExistingAssessments = assessments;