A Couple of Suggestions from Experience

Jun 13, 2014 at 3:06 AM
Hi Le,

Great work here. Probably the best open source repository implementation around!

I have managed to shoe-horn it into the infrastructure for a sample application which I am building. I have a ProviderFactory which creates repositories. So, in my Unit of Work, I’ve slightly modified your method to look like this:
public IRepositoryAsync<TEntity> RepositoryAsync<TEntity>() where TEntity : class, IObjectState
{
    if (_repositories == null)
    {
        _repositories = new Dictionary<string, dynamic>();
    }

    var type = typeof(TEntity).Name;

    if (_repositories.ContainsKey(type))
    {
        return _repositories[type];
    }

    _repositories.Add(type, RepositoryProvider.GetRepositoryForEntityType<TEntity>());

    return _repositories[type];
}
The RepositoryProvider removes the need for casting when adding to the _repositories dictionary and I changed the value to have type of dynamic, rather than object. Again, no explicit casting required anymore.

The other change I had to make involved generic constraints. In some of your classes (most notably, Repository), you are using a constraint that the generic member has to be an Entity. I changed that such that the constraint is now that it has to be a reference type which implements IObjectState i.e. where Tentity : class, IObjectState

I’m not sure what your thoughts are on that and whether there is a reason you are forcing the constraint to be the Entity class. If there is a good reason, by all means reply here as I’m always eager to learn new things. I had to make that change to work with my factories. Otherwise, it just wouldn't work.

Happy to let you see my code. Just say the word and I’ll give a link.
Jun 18, 2014 at 10:15 AM
Hi dave,

Can you please share the link of how you are using RepositoryProvider ?
Jun 18, 2014 at 10:32 AM
Yeah for sure. Basically, it’s just a Winforms app which demonstrates how I would design an enterprise CRUD application. Nowhere near complete, but the Lion’s share of the plumbing is in place.

The repository is here.

More Specifically, the factories are contained in this folder
The UnitOfWork in here

Also, as I mentioned in my original post, I replaced generic constraints of Entity anywhere in Le’s library to where Tentity : class, IObjectState.
Here is an example of that.

Cheers
Jun 18, 2014 at 11:04 AM
Edited Jun 18, 2014 at 11:04 AM
daverogers,

Thanks for the feedback and suggestions, would you mind doing a pull request and incorporating changes in this? I would like to review this, if it works out, I may merge this into the release.
Jun 19, 2014 at 12:46 AM
Yep. No worries. Will add the code this week with pull request.

I should note that the code originated from a John Papa course. I stumbled across it in various Stackoverflow questions (so it is well and truly in the public domain).
Jul 2, 2014 at 9:41 PM
Edited Jul 2, 2014 at 9:41 PM
Update, your pull request has been merged, with the unit and integration tests updated for RepositoryProvider. There is one place I'd like for you to review https://genericunitofworkandrepositories.codeplex.com/SourceControl/latest#main/Source/Repository.Pattern.Ef6/UnitOfWork.cs, which is commented daverogers.
Jul 3, 2014 at 8:35 AM
Hi Le,

Not sure if you received my previous email, so I’ve dropped the text of it at the end of this post. But in addition to that, I found something else today which causes an issue between the factories and the repositories.

The first method in the RepositoryFactories class, GetFactories, provides a way of adding custom repositories for types. With the overall solution structure, however, if one were to attempt to add a custom repository to that dictionary, then they would create a circular reference between projects i.e. the Entities project and the Repository.Pattern.EF6 project. I’ve never run into this as I have never attempted to create a repository which wasn’t the default repository.

This doesn’t sink the whole thing. But a tutorial would need to be written to explain that feature - the two main things that would have to happen are that the custom repository would need to sit in a separate project (custom repositories) and the entities would need to either implement IObjectState directly, or from a parent class which sits in the same project as the Entities classes.

Big apologies for not picking this up. I wanted to bring to your attention ASAP before you released it.
If I seem a bit disorganized, it's because I'm home sick with a head full of flu medication.

Previous email today:
Hi Le,

You are absolutely correct. The repositories are cached in the factory making the unit of work’s cache redundant.

I noticed something else while I was reviewing the code. I've now replaced all of the return types which included the "object" type with "dynamic" e.g. private readonly IDictionary<Type, Func<IDataContextAsync, IUnitOfWorkAsync, dynamic>> _repositoryFactories;

With that change, there is no need to cast the repositories in the GetRepository and MakeRepository methods.

However, the code runs fine when it is just "object". But I like to use the dynamic run-time instead of the object type. So go with whichever one your happy with. I’ve pushed my changes to the fork. It is just the 3 files.

Sorry I did not pick that up in my first pull request.

Regards
David
Jul 3, 2014 at 8:52 AM
Sorry, scratch my last post, there will always be a circular reference.

GetFactories needs to be changed such that it returns a new empty dictionary.
Jul 3, 2014 at 3:52 PM
Hey Dave, thanks for following up because I didn't receive your previous for some reason (email: LeLong37@gmail.com). Anyhow, this could be mitigated if we just removed the custom repository feature for now correct? Because I do want to keep the repo factory and provider and I prefer the concern of creating factories be out of the UnitOfWork.cs is what we have now.

As for the custom repos feature, we can hibernate the feature for now. I do want to put some more thought around it. I wanted it to be more generic, right now it looks like it supports repos that are passed in a IDataContext & IUnitOfWork. Is there a way to make this more generic so when we get this figured out we could add a repo of any type? Meaning if we had a repository of IAzureBlobRepository<Blob> which has nothing to do with our IDataContext we could add it. If this isn't possible, that's fine we could just have an IAzureBlogService<Blob> which wouldn't need a repository, thoughts?
Jul 3, 2014 at 6:12 PM
@Dave

Didn't see the 3 files you pushed from object to dynamic in your branch?
Jul 3, 2014 at 6:20 PM
Edited Jul 3, 2014 at 7:19 PM
disregard the last message, It's obvious enough where to change from object to dynamic without having visibility to your push https://genericunitofworkandrepositories.codeplex.com/SourceControl/changeset/d0523c849edc8b43b8a95b7b3851323e01b0beb4, let me know if you see anything that shouldn't be.
Jul 4, 2014 at 5:50 AM
Hi Le, I confirm that code is all correct. I agree with respect to putting off the custom repository feature for the time being. Once I get over my flu and can think a bit clearer, I'll digest your suggestion and pursue it.

Have a good one!
Marked as answer by lelong37 on 7/7/2014 at 8:45 AM