List<T> item remove problem.

Question

Don't be scared of the extensive code. The problem is general. I just provided the code to understand the problem better.

I am trying to find out a standard approach of manipulating tables with many-to-many relationships. And I am almost done. Here Teacher and Course have M:M relationship. I have designed my classes as follows:

Teacher - class:

public class Teacher
{
    public int ID{get;set;}
    public string TeacherName{get;set;}
    private List<Course> _items = null;
    public List<Course> Items
    {
     get 
     { if (_items == null) {_items = Course.GetCoursesByTeacherID(_ID);}
      return _items;
     }
     set {_items = value;}
    }
    public int Save() 
    { //...
     CourseTeacher.DeleteCoursesByTeacherID(tc, id);
        CourseTeacher.SaveCoursesWithTeacherID(tc, id, this.Items);
        //...
    }
    public bool Update()
    { //...
     CourseTeacher.DeleteCoursesByTeacherID(tc, this.ID);
        CourseTeacher.SaveCoursesWithTeacherID(tc, this.ID, this.Items);
     //...
    }
    public static Teacher Get(int id)
    { //...
     item.Items = CourseTeacher.GetCoursesByTeacherID(tc, item.ID);//...
    }
    public static List<Teacher> Get()
    { //...
     items[i].Items = CourseTeacher.GetCoursesByTeacherID(tc, items[i].ID);//...
    }
    public static List<Teacher> GetTeachersByCourseID(int id)
    { //...
     items = CourseTeacher.GetTeachersByCourseID(tc, id);//...
    }
    public bool Delete()
    { //...
     CourseTeacher.DeleteCoursesByTeacherID(tc, this.ID);//...
    }
}

Course is absolutely similar to Teacher - class. And the mapping class is as follows:

public class CourseTeacher
{
    public int CourseID{get;set;}
    public int TeacherID{get;set;} 
    public static void SaveCoursesWithTeacherID(TransactionContext tc, int teacherID, List<Course> items){}
    public static void SaveTeachersWithCourseID(TransactionContext tc, int courseID, List<Teacher> items){}
    private void Save(TransactionContext tc){}
    public static void DeleteCoursesByTeacherID(TransactionContext tc, int teacherID){}
    public static void DeleteTeachersByCourseID(TransactionContext tc, int courseID){}
    public static List<Teacher> GetTeachersByCourseID(TransactionContext tc, int courseID){}
    public static List<Course> GetCoursesByTeacherID(TransactionContext tc, int teacherID){}
}

Now my problem is, this code is not working?

Teacher professorXyz = Teacher.Get(2);                        
Course cpp = Course.Get(3);
Course java = Course.Get(2);
professorXyz.Items.Remove(cpp);
professorXyz.Items.Remove(java);
professorXyz.Update();

This is not working because it is not probably finding a match or get accessor is returning readonly List.

How should I refactor my Teacher/Course - class to achieve this?

No exception. No problem with persistence code. Items are not being removed.

why professorXyz.Items.Contains(cpp); is returning false?

What to check for?

Answer 1

What are you trying to do? Your sample looks like you want to build a relational database table implemented in C#.

If you want to have an OO representation then get rid of the entire CourseTeacher class. That has absolutely nothing to do with OO.

Answer 2

How about adding and removing done within the Teacher Class?

public class Teacher
{
        //.... Original implementations
    public bool AddCourse(Course course) {
     if(_items.Contains(course)) return false;

     _items.Add(course);
     return true;
    }

        // similarly for remove course

}

Answer 3

This is not a direct answer, but...

Your design is very (very) Relational. That makes persisting to a DB easier but you do not have a proper OO model. Maybe you should consider using DataTables in a DataSet and reap the benefits of the Relation class.

---

To take a shot:

Teacher professorXyz = Teacher.Get(2);                        
Course cpp = Course.Get(3);

I suspect that the cpp course is being loaded twice, and that there are 2 instances of that course in memory. A very bad consequence of your design. By default, those 2 instances will not be equal and that is why Remove does not work. You could overload Equals, == and GethashCode but that is not recommended for mutable types.
What you really need is a design where for a given Teacher or Course there never exists more than 1 instance in memory.

Re Comment: A MxM relation in OO looks like:

class Teacher
{
   public readonly List<Course> Courses = ...;
}

class Course
{
   public readonly List<Teacher> Teachers = ...;
}

This will take a little more work to write to a DB but it solves a lot of other problems.

Answer 4

Hi JMSA,

seems you already solved this problem, but consider following code where I overrode bool Equals; C# couldn't knew how to compare your new cpp instance with another instance in your List<Course>, so we need to tell it by creating a more specialized Equals method:

class Teacher
{
    private List<Course> items = new List<Course>();

    public int ID { get; set; }
    public List<Course> Items { get { return items; } }
}

class Course
{
    public int ID { get; set; }

    public override int GetHashCode()       { return base.GetHashCode(); }
    public override bool Equals(object obj) { return Equals(obj as Course); }
    public bool Equals(Course another)
    {
        return another != null && this.ID.Equals(another.ID);
    }
} 

static void Main(string[] args)
{
    Teacher teacher = new Teacher { ID = 2 };
    teacher.Items.AddRange(
        new Course[] { 
            new Course{ ID = 2 },       // java
            new Course{ ID = 3 } });    // cpp

    Course cpp = new Course { ID = 3 }; // previous problem: another instance
    teacher.Items.Contains(cpp);        // now returns true
    teacher.Items.Remove(cpp);          // now returns true
}

Answer 5

Henk is correct; your design is very, very relational. For this sort of scenario, though, you're better off focusing on behaviour in your objects, and using an object-relational mapping (ORM) tool to translate between your objects and your database.

ADO.NET's DataTable and DataSet don't really offer object-relational mapping capabilities; because they're so tightly coupled to the underlying database schema, they force you to think in terms of columns, tables and relations, when you really want to be thinking in terms of teachers and courses.

I would seriously recommend looking at Castle ActiveRecord for this scenario. It uses the same approach as your example - static Teacher.Get() to retrieve an instance, myTeacher.Save() to save your changes - but there's a LOT of necessary complexity that your example is missing, and using an ORM framework will allow you to ignore this complexity and focus on your own project's requirements.

Here's an example of many-many associations from the Castle ActiveRecord documentation that you may find helpful.