Unit of Work with Generic Repository Pattern MVVM, vol. 3

Question

Continuation of this question: https://codereview.stackexchange.com/questions/36561/unit-of-work-with-generic-repository-pattern-mvvm-vol-2

I have made some modifications:

• Implement lazy-loading (hope so)
• Add() and Update() returns T instead of void
• Add IFeedItemRepository, FeedItemRepository, IBaseRepository<T>

IBaseRepositry

public interface IBaseRepository<T> where T : class
{
    int Count { get; }
    T Add(T entity);
    T GetById(int id);
    T Update(T entity);
    IEnumerable<T> GetAll();
    void Remove(T entity);
    void RemoveById(int id);
}

IBaseFeed

public interface IBaseFeed
{
    int Id { get; set; }
    string Title { get; set; }
    DateTime PubDate { get; set; }
    Uri Link { get; set; }
    string Misc { get; set; }
}

IFeedRepository

internal interface IFeedRepository<T> : IBaseRepository<T> where T : class, IBaseFeed
{
    int GetFeedIdByLink(string feedLink);
    int GetFeedIdByLink(Uri feedLink);
    T GetByLink(string feedLink);
    T GetByLink(Uri feedLink);
}

FeedRepository

internal class FeedRepository<T> : IFeedRepository<T>, IDisposable
    where T : class, IBaseFeed, new()
{
    private readonly SQLiteConnection _db;
    private Lazy<IList<T>> _feeds;

    private bool _isDisposed = false;

    public int Count
    {
        get
        {
            return _feeds.Value.Count;
        }
    }

    public FeedRepository(SQLiteConnection db)
    {
        this._db = db;
        this._feeds = new Lazy<IList<T>>(() => _db.Table<T>().ToList());
    }

    public T Add(T feed)
    {
        this._db.Insert(feed);
        return feed;
    }

    public IEnumerable<T> GetAll()
    {
        return this._feeds.Value;
    }

    public T GetById(int id)
    {
        return this._feeds.Value.Where(feed => int.Equals(feed.Id, id)).Single();
    }

    public T GetByLink(string feedLink)
    {
        return this.GetByLink(new Uri(feedLink));
    }

    public T GetByLink(Uri feedLink)
    {
        return this._feeds.Value.Where(feed => Uri.Equals(feed.Link, feedLink)).Single();
    }

    public int GetFeedIdByLink(string feedLink)
    {
        return this.GetFeedIdByLink(new Uri(feedLink));
    }

    public int GetFeedIdByLink(Uri feedLink)
    {
        return this._feeds.Value.Where(feed => Uri.Equals(feed.Link, feedLink)).Select(feed => feed.Id).Single();
    }

    public void Remove(T feed)
    {
        this._db.Delete(feed);
    }

    public void RemoveById(int id)
    {
        this.Remove(this.GetById(id));
    }

    public T Update(T feed)
    {
        this._db.Update(feed);
        return feed;
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool isDisposing)
    {
        if (!this._isDisposed)
        {
            if (isDisposing)
            {
                if (this._db != null)
                {
                    this._db.Dispose();
                }
            }

            this._isDisposed = true;
        }
    }

IFeedItemRepository

internal interface IFeedItemRepository : IFeedRepository<FeedItem>
{
    IEnumerable<FeedItem> GetFeedItemsByFeedDataId(int feedDataId);
}

FeedItemRepository

internal sealed class FeedItemRepository : FeedRepository<FeedItem>, IFeedItemRepository
{
    public FeedItemRepository(SQLiteConnection db)
        : base(db) { }

    public IEnumerable<FeedItem> GetFeedItemsByFeedDataId(int feedDataId)
    {
        return base.GetAll().Where(feed => int.Equals(feed.FeedDataId, feedDataId));
    }
}

IUnitOfWork

internal interface IUnitOfWork
{
    void Commit();
    void Rollback();
}

UnitOfWork

public class UnitOfWork : IUnitOfWork, IDisposable
{
    private readonly SQLiteConnection _db;

    private IFeedRepository<FeedData> _feedDataRepository;
    private IFeedItemRepository _feedItemRepository;

    private bool _isDisposed = false;

    public IFeedRepository<FeedData> FeedDataRepository
    {
        get
        {
            return _feedDataRepository;
        }
        set
        {
            _feedDataRepository = value;
        }
    }

    public IFeedItemRepository FeedItemRepository
    {
        get
        {
            return _feedItemRepository;
        }
        set
        {
            _feedItemRepository = value;
        }
    }

    public UnitOfWork(SQLiteConnection db)
    {
        this._db = db;
        this._feedDataRepository = new FeedRepository<FeedData>(this._db);
        this._feedItemRepository = new FeedItemRepository(this._db);

        this._db.BeginTransaction();
    }

    public void Commit()
    {
        this._db.Commit();
    }

    public void Rollback()
    {
        this._db.Rollback();
    }

    protected virtual void Dispose(bool isDisposing)
    {
        if (!this._isDisposed)
        {
            if (isDisposing)
            {
                if (this._feedDataRepository != null)
                {
                    ((FeedRepository<FeedData>)this._feedDataRepository).Dispose();
                }

                if (this._feedItemRepository != null)
                {
                    ((FeedItemRepository)this._feedItemRepository).Dispose();
                }

                if (this._db != null)
                {
                    this._db.Dispose();
                }
            }

            this._isDisposed = true;
        }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

Has the implementation of lazy-loading been done correctly?

What about this (UoW):

    private IFeedRepository<FeedData> _feedDataRepository;
    private IFeedItemRepository _feedItemRepository;

Is it correct?

Is the constructor in FeedItemRepository done properly?

Answer 1

Few tiny items:

1. Can you use an IDbConnection interface (or worst case, DbConnection class) instead of SQLiteConnection for your constructor parameter/class member? This would allow for more generic operation, easier unit testing and easier swap-out of database providers in the future. This is in both the UnitOfWork class and the FeedRepository<T> class. Upon further reflection, the way that particular SQLite client library is used, this is unfeasible at this time. Still, I'd recommend a different SQLite client if at all possible which conforms to IDbConnection, etc.

2. I would advise against Dispose()ing of the _db in the UnitOfWork class' Dispose() method since the class doesn't create it - the caller does. Let the caller Dispose() of it. Same goes for the FeedRepository<T> class.

3. Do the FeedDataRepository and FeedItemRepository properties in the UnitOfWork class really need setters? It seems dangerous to create them in the constructor and then Dispose() them in the Dispose() method when someone could come along and change either to something else entirely. You could lose the original references altogether. I'd wager these should be invariant once constructed.