Creating and populating a generic list

Question

I would like to know if this is being done correctly.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Data;
using System.Diagnostics;
using System.Linq;
using System.Text;
public class InnerJoinTables
{
 public List<InnerJoinTable> Items = new List<InnerJoinTable>();
}
public class InnerJoinTable
{
 public int ID { get; set; }
 public string RightTable { get; set; }
 public string LeftTable { get; set; }
 public InnerJoinTable(int ID, string RightTable, string LeftTable)
 {
 this.ID = ID;
 this.RightTable = RightTable;
 this.LeftTable = LeftTable;
 }
}

Calling code:

InnerJoinTables m = new InnerJoinTables();
m.Items.Add(new InnerJoinTable(1, "A", "B"));

• svick: Does it seem to work? What exactly are you worried about? Why do you think it might not be the correct way?
• Leonid: You can achieve the same result using a stored procedure, an ORM or LINQ. What do you find the need to implement this join yourself?

Answer 1

The code will work, so from that point of view, it is correct.

But there are also some issues with it:

1. I don't see any reason for the InnerJoinTables class to exist. You should just use List<InnerJoinTable> directly in the calling code. That's unless there's something else going on that you're not showing.
2. Public fields should not be used. Use a property instead.

Answer 2

You can directly derive your InnerJoinTables class with List. Then you can use it as normal list object. No need of having items as member there.

• Brian: This is not necessarily a good idea. I prefer composition over inheritance, given a choice.
• svick: Could you explain how is that better than using List directly?
• Vivek Dixit: It just change the way you would use that object in your code. If you have an InnerJoinTables and Items as collection of InnerJoinTable object. Each time you want to access the object you would write innerJoinTables.Items.Add(). If you just have it derived with list then it would be like innerJoinTables.Add(new InnerJoinTable()), that's why I thought its better than using list directly.