What is a better, cleaner way of using List

Question

I'm looking to implement a few nicer ways to use List in a couple of apps I'm working on. My current implementation looks like this.

MyPage.aspx.cs

protected void Page_Load(object sender, EventArgs e)
{
    BLL.PostCollection oPost = new BLL.PostCollection();
    oPost.OpenRecent();
    rptPosts.DataSource = oArt;
    rptPosts.DataBind();
}

BLL Class(s)

public class Post
{
    public int PostId { get; set; }
    public string PostTitle { get; set; }
    public string PostContent { get; set; }
    public string PostCreatedDate { get; set; }

    public void OpenRecentInitFromRow(DataRow row)
    {
        this.PostId = (int) row["id"];
        this.PostTitle = (string) row["title"];
        this.PostContent = (string) row["content"];
        this.PostCreatedDate = (DateTime) row["createddate"];
    }
}
public class PostCollection : List<Post>
{
    public void OpenRecent()
    {
        DataSet ds = DbProvider.Instance().Post_ListRecent();
        foreach (DataRow row in ds.Tables[0].Rows)
        {
            Post oPost = new Post();
            oPost.OpenRecentInitFromRow(row);
            Add(oPost);
        }
    }
}

Now while this is working all well and good, I'm just wondering if there is any way to improve it, and just make it cleaner that having to use the two different classes do to something I think can happen in just one class or using an interface.

Answer 1

For one thing, I wouldn't derive from List<T> - you aren't really specializing the behaviour.

I'd also suggest that you could make Post immutable (at least externally), and write a static method (or constructor) to create one based on a DataRow:

public static Post FromDataRow(DataRow row)

Likewise you can have a list method:

public static List<Post> RecentPosts()

which returns them. Admittedly that might be better as an instance method in some sort of DAL class, which will allow mocking etc. Alternatively, in Post:

public static List<Post> ListFromDataSet(DataSet ds)

Now, as for the use of List<T> itself - are you using .NET 3.5? If so, you could make this considerably neater using LINQ:

public static List<Post> ListFromDataSet(DataSet ds)
{
    return ds.Tables[0].AsEnumerable()
                       .Select(row => Post.FromDataRow(row))
                       .ToList();
}

Answer 2

Edit: John Skeet's answer is probably a better option. But if you want to make just a few simple changes, read on:

Place the database access code, OpenRecentInitFromRow into the PostCollection and treat that as a Post manager class. That way the Post class is a plain old Data Transfer Object.

public class Post
{
    public int PostId { get; set; }
    public string PostTitle { get; set; }
    public string PostContent { get; set; }
    public string PostCreatedDate { get; set; }
}

public class PostCollection : List<Post>
{
    public void OpenRecent()
    {
        DataSet ds = DbProvider.Instance().Post_ListRecent();
        foreach (DataRow row in ds.Tables[0].Rows)
        {
            Add(LoadPostFromRow(row));
        }
    }

    private Post LoadPostFromRow(DataRow row)
    {
        Post post = new Post();
        post.PostId = (int) row["id"];
        post.PostTitle = (string) row["title"];
        post.PostContent = (string) row["content"];
        post.PostCreatedDate = (DateTime) row["createddate"];
        return post;
    }
}

Answer 3

Are you deriving from List<T> because you want to offer other consumers of PostCollection the ability to Add and Remove items? I'm guessing not, and that you actually just want a way to expose a collection you can bind to. If so, you could consider an iterator, perhaps:

class BLL {
    ...

    public IEnumerable<Post> RecentPosts {
        get {
            DataSet ds = DbProvider.Instance().Post_ListRecent(); 
            foreach (DataRow row in ds.Tables[0].Rows) 
            { 
                Post oPost = new Post(); 
                oPost.OpenRecentInitFromRow(row); 
                yield return oPost;
            } 
        }
    }    

    ...
}

Notwithstanding the fact that this might be considered poor form (in that we have a property getter that might be making a network call), this iterator approach will do away with the overhead of calling OpenRecentInitFromRow for Posts that are never enumerated.

You also become agnostic as to how potential consumers of your Posts might want to consume them. Code that absolutely, positively has to have every Post can do ToList(), but other code might want to use a LINQ query that short-circuits the enumeration after the right Post is found.

Answer 4

I'm looking to implement a few nicer ways to use List

That seems like an odd request. The "List" type is a means, rarely an end. With that in mind, one nicer way to accomplish your real end is to use IEnumerable rather than List, because that List forces you to keep your entire collection in memory while IEnumerable only requires one object at a time. The trick is just that you have to wire everything in your processing stream, from the data layer all the way up through presentation, to use it.

I have a good example in the link below about how to do this in a very clean way:
Fastest method for SQL Server inserts, updates, selects

Depending on your existing data layer code you may be able to skim much of the first half of the (long) post - the main point is that you use an iterator block to turn an SqlDataReader into an IEnumerable<IDataRecord>. Once you have that, it's pretty straightforward the rest of the way through.

Answer 5

You could do this:

protected void Page_Load(object sender, EventArgs e)
{
    BLL.PostCollection oPost = new BLL.PostCollection();
    rptPosts.DataSource = Post.OpenRecent();
    rptPosts.DataBind();
}
public class Post
{
    public int PostId { get; set; }
    public string PostTitle { get; set; }
    public string PostContent { get; set; }
    public string PostCreatedDate { get; set; }

    public void OpenRecentInitFromRow(DataRow row)
    {
        this.PostId = (int) row["id"];
        this.PostTitle = (string) row["title"];
        this.PostContent = (string) row["content"];
        this.PostCreatedDate = (DateTime) row["createddate"];
    }

    public static List<Post> OpenRecent()
    {
        DataSet ds = DbProvider.Instance().Post_ListRecent();
        foreach (DataRow row in ds.Tables[0].Rows)
        {
            Post oPost = new Post();
            oPost.OpenRecentInitFromRow(row);
            Add(oPost); //Not sure what this is doing
        }
        //need to return a List<Post>
    }
}