Return to Answer

###Leaky abstractions

A specific implementation should not leak into an abstraction. By making IUnitOfWork extend IDisposable, that's what you're doing:

public interface IUnitOfWork : IDisposable

The goal of the abstraction, is to abstract implementation details away - in this case, the fact that you're using NHibernate. You had a specific implementation in mind when you created this interface, and that implementation leaked into the abstraction. What if you wanted to implement an IUnitOfWork that worked off a List<T>, for testing purposes?

Not all implementations of IUnitOfWork need to implement IDisposable - better leave that up to the implementation:

public class NHibernateUnitOfWork : IUnitOfWork, IDisposable
{
 private readonly ISession _session;
 /* implementation */
 public void Dispose()
 {
 _session.Dispose();
 }
}

public class TestUnitOfWork : IUnitOfWork
{
 /* implementation */
}

###IQueryable

Every single implementation of the Repository & Unit of Work patterns I have seen, were different. Every single one.

Without seeing what your implementations and client code looks like (/how you're using these interfaces), it's very hard to tell whether you've done something wrong.

For one thing, exposing IQueryable<T> is leaking the query outside your repository, making it possible for client code to call IRepository<T>.FindAll() and then from there, add a Where clause or Join on another repository, which means the repository completely loses control of query materialization, which [unless I missed something] defeats its purpose.

By exposing a IRepository<T> GetRepository<T>() method on your IUnitOfWork, you're also making it possible for the unit of work's clients to own the materialization of the query.

Whether that's good or bad, entirely depends on your architecture; I cannot tell just from the interfaces... I can only raise a flag.

Leaky abstractions

A specific implementation should not leak into an abstraction. By making IUnitOfWork extend IDisposable, that's what you're doing:

public interface IUnitOfWork : IDisposable

The goal of the abstraction, is to abstract implementation details away - in this case, the fact that you're using NHibernate. You had a specific implementation in mind when you created this interface, and that implementation leaked into the abstraction. What if you wanted to implement an IUnitOfWork that worked off a List<T>, for testing purposes?

Not all implementations of IUnitOfWork need to implement IDisposable - better leave that up to the implementation:

public class NHibernateUnitOfWork : IUnitOfWork, IDisposable
{
 private readonly ISession _session;
 /* implementation */
 public void Dispose()
 {
 _session.Dispose();
 }
}

public class TestUnitOfWork : IUnitOfWork
{
 /* implementation */
}

IQueryable

Every single implementation of the Repository & Unit of Work patterns I have seen, were different. Every single one.

Without seeing what your implementations and client code looks like (/how you're using these interfaces), it's very hard to tell whether you've done something wrong.

For one thing, exposing IQueryable<T> is leaking the query outside your repository, making it possible for client code to call IRepository<T>.FindAll() and then from there, add a Where clause or Join on another repository, which means the repository completely loses control of query materialization, which [unless I missed something] defeats its purpose.

By exposing a IRepository<T> GetRepository<T>() method on your IUnitOfWork, you're also making it possible for the unit of work's clients to own the materialization of the query.

Whether that's good or bad, entirely depends on your architecture; I cannot tell just from the interfaces... I can only raise a flag.