After-school service for students

I have to show a select control of students with event information
where Monday, Tuesday, Wednesday, Thursday and Friday are Events. (Independent buttons). The second columns are Events too, but you can add these Events using the "Add" button.

Data flows in like this:

I have to maintain the state the state of every student, and I thought that I would create a simple structure like this:

Note: I have implemented a little ShopCart just to make the code works.

DTOs

public class AfterSchoolProgramInformation
{
    public int TotalShopCartItems { get; set; }
    public IEnumerable StudentInfo { get; set; }
    public IEnumerable ShopCartInfo { get; set; }
}
public class StudentInfo
{
    public int StudentId { get; set; }
    public String Name { get; set; }
    public IEnumerable DayInfo { get; set; }
}
public class DayInfo
{
    public string DayName { get; set; }
    public decimal Fee { get; set; }
    public bool Added { get; set; }       
    public int EventId { get; set; }
    public IEnumerable DayInfoDetail { get; set; }
}
public class DayInfoDetail
{
    public string ClassName { get; set; }
    public decimal Fee { get; set; }
    public bool Added { get; set; }
    public int EventId { get; set; }
}
public class ShopCartInfo
{
    public int StudentId { get; set; }
    public int[] EventIds { get; set; }
}

DataClass

```
public class AfterSchoolService
{
public IEnumerable GetAfterSchoolInformation()
{
DataTable result = new FakeDatabase().GetAfterSchoolInformation();

var dayInformation = (from data in result.AsEnumerable()
group data by new
{
DayWeek = data.Field("DayOfWeek"),
DayEventId = data.Field("DayEventID"),
DayFee = data.Field("DayFee")
}
into provisional

Why did you write that GetValue DictionaryExtension when Dictionary.TryGetValue already exists?

I'm not a fan of properties like IEnumerable StudentInfo where the name of the property doesn't reflect that it's an IEnumerable. Sure, StudentInfos is not exactly great, which probably means the class name StudentInfo isn't that great and perhaps should change. Quite frankly I can't see why that class isn't simply called Student?

Another naming issue: from data in result.AsEnumerable(): data is a plural and isn't telling me anything about what it contains.

Also, why does new FakeDatabase().GetAfterSchoolInformation() return a DataTable which you then immediately have to convert using .AsEnumerable()? Avoid working with the likes of DataTable and instead convert data from your database to custom classes as soon as possible. Why not use an ORM like Entity Framework or NHibernate etc.? That way you also don't need to have ugly code like data.Field("DayOfWeek") (yay, magic strings!).

Also, be consistent in naming : DayWeek = data.Field("DayOfWeek"). I would expect the database field DayOfWeek to map to the property DayOfWeek, instead it's called DayWeek.

I have no words for this:

.OrderBy(s => s.DayName == "Fr")
.ThenBy(s => s.DayName == "Th")
.ThenBy(s => s.DayName == "We")
.ThenBy(s => s.DayName == "Tu")
.ThenBy(s => s.DayName == "Mo")

It's obvious DayName doesn't contain the name, but instead an abbreviation, so the property is named incorrectly. Which you then "fix" by doing this: s.DayName = Utils.GetDayNameBySuffix(s.DayName);.

And this sort is just... Why not have a numeric field -- e.g. DayId -- and sort on that?

You should move a lot of code from Home to one or more separate classes. Code like this just isn't right when you're in the Home class already:

Home instance = new Home();
instance.RemoveEventFromCart(eventId, studentid);

Avoid magic strings. Things like the names of session variables -- e.g. "Cart" -- should be constants, preferably in a separate class.

_ShopCartItems is a List so you could use its Count property, not the extension method Count();

Why is EventIds on ShopCartInfo an array and not an IEnumerable?

Quite frankly your whole structure feels unwieldy and needlessly hierarchical. Why is there a Fee on DayInfo and also on DayInfoDetail? Ditto EventId. StudentId is both in StudentInfo and ShopCartInfo. To me it feels like this structure should be more about relationships between various objects.

I would also encourage you to separate your layers, e.g. have a data layer that deals with the db, have a business layer where you keep all the logic, have a presentation layer that is as slim as possible,...

StackExchange Code Review Q#96837, answer score: 8