Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What is a better, cleaner way of using List<T>

Tags:

c#

list

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.

like image 249
Tim Meers Avatar asked May 25 '10 13:05

Tim Meers


3 Answers

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();
}
like image 86
Jon Skeet Avatar answered Nov 28 '22 16:11

Jon Skeet


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.

like image 21
lesscode Avatar answered Nov 28 '22 14:11

lesscode


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;
    }
}
like image 44
Daniel Dyson Avatar answered Nov 28 '22 14:11

Daniel Dyson