implemented unitofwork ref #16

[toeb]

This takes away the DBContext savechanges calls and extracts them away from the business layer. Its good

[toeb]

We should have a document / explanaition of how to use the data layer

[matthiaszoellner]

My concern is, that IUnitOfWork and its implementation as it is now, has to provide a getter for each used data repository, meaning we get two additional points of maintainance for each table.

As far as I see, we didn't have that with the Repository / DbContext, where the relevant repository was determined by generic parameters.

I would prefer a similar flexibility in the unit of work, instead of explicit property design for each single table. The underlaying logic to the getter (UnitOfWork.cs:31) is generic anyway, so it should be possible.

[toeb]

I agree. I think a solution would be to let IUnitOfWork only have a Save method which wraps DbContext.SaveChanges() in its implementation and the command handlers can import their repositories and IUnitOfWork in properties. Also if the commands do not need to savechanges they do not need to import IUnitOfWork.

[toeb]

I have proposed another pull request based on this one which removes the repositories from unit of work as stated before

[matthiaszoellner]

Well now that destroys the whole unit of work principle... you could mix up a database-repository with a mock unit of work - good luck saving

I'm still thinking about the details, but it goes along this way:

• replacing the getter property in unit of work with a generic getter function
• adding some type-independent storage (list of objects or such stuff) in place of accountRepo
• doing some casting magic in the getter to select the requested repository from storage
• creating a new instance and adding to storage, if not found

Goal should be to have a mockable unit of work (why else use it at all?) which we can use for any table without touching it again.

[matthiaszoellner]

Implemented the proposed change - any real drawbacks to this solution?

[toeb]

Mocking would still be possible without adding repository access to iunitofwork. (It would also be much easier) because now you have to mock a generic method instread of injecting a mocked repository into the composition container. Also I find the code alot nicer if you have a reference to the repository in the class instead of doing a getrepository<> call in an importing contructor. UnitOfWork.GetRespository().Get().... is not really helping the code.

Also the unitofwork is actually the composition container, unitofwork could also be called PersistService which has a single Save method.

[flom]

I am not sure whether a generic getter is the right approach. We only have a fixed number of repositories but a generic interface implies that we can create repositories for any class. Having fixed properties might be more effort when new model/repository are introduced but this shouldn't happen to often. Also with fixed properties it is easier to return specialized repositories instead of returning RepositoryBase for each model class. It might possible to have a ReversibleRepository which is a subclass of RepositoryBase but is just used for the class Account (or more general: a subset of all model classes).

[toeb]

The last point you mentioned is exactly what the composition container ie the unit of work does. Instead of adding repository base you can export specialized repositories where necessary or exchange the repository base class itself with e.g. a ReversibleRepository

[matthiaszoellner]

Actually, the IUnitOfWork interface as I proposed doesn't say anything about the Repository implementation - it returns IRepository<T> where T is a subclass of Meta (could be anything, but I think its reasonable to restrict it to Meta). Only the UnitOfWork implementation - as it is now - creates and returns RepositoryBase<T> objects and should kindly remind you with some sort of exception/error, if T is not available in datalayer. It is still possible in this or another UnitOfWork implementation, to return any other class which is a subclass of IRepository<T>. So ReversibleRepository<T> could be enabled by only changing the GetRepository<T> method - possibly distinguishing the repository creation by T

[toeb]

So what do we do? The last proposed solution is the compromise.

[toeb]

anybody against?

[matthiaszoellner]

Look at the following link, if we want more flexibility, regarding the specific repository type. http://stackoverflow.com/questions/16064902/dependency-injection-in-unit-of-work-pattern-using-repositories

It is even closer to the goal of extendibility without touching the code again after first implementation.

But since I dont see us having a lot of different repository subclasses in use, I think the current state is fine

[toeb]

Ok I;ll just merge this now so we can continue to work. later on we might come back to this