51

I've never TDD'd, but don't do that: public class MyDataContext<T> : DbContext where T : class This gives you a context-per-entity, which might work for ultra-simplistic CRUD scenarios, but doesn't scale very well and will quickly give you headaches as soon as you need to deal with more than a single entity type in a single transaction - because that'...


25

You should use Expression<Func<T, bool>> as predicates on your interface and have the RepositoryBase implement that interface. By only using Func, you won't get translation to L2E etc, but will have to enumerate the entire DB table before you can evaluate the Func. The interface can be mocked, hence unit-tested without a physical db and also used ...


24

Why bother checking if the value/property has been changed? You are just adding additional complexity for no reason. Sure, it might be slightly! faster to only update modified values, but in return you are adding an overhead to keep track of the modified values, which will diminish whatever performance boost you got. Just invoke Update on the entire object....


23

EDIT: I apologize from my earlier sample that didn't quite compile. I've fixed it and added a more complete example. You could associate each condition with a strategy for changing the query. Each strategy (called SearchFieldMutator in this example) will hold two things: A way to decide if to apply the strategy. The strategy itself. The first part is a ...


20

You're doing it right - per-request lifestyle is exactly what you need, and you got it; this ensures your favorite IoC container disposes your context at the end of each request, and serves a new instance at each new one. One thing bothers me though, and it's not about your code: Use UnitOfWork and Repository patterns seems to be overkill because whole ...


20

I'd second TopinFrassi's answer, that this is a more or less sensible approach, and the implementation is excellent. However, there are some potential issues, which I'll expand from craftworkgames' comments and your response. Because I can't really find anything that needs criticism in how it's written, this will be entirely design focussed. There are two ...


17

Have a look on the Onion Architecture series by Jeffrey Palermo How about this for a structure Project.Domain (Class Library) has no deppendencies on any other projects represents the core business of your app/service any external dependecies are abstracted away via DI (adapter pattern) you might want to define an IDataContext/ISession that abstracts ...


15

I know this is old, but thought it may be helpful for anyone reading this. If you want to clean up the code, you can always refactor it.. something like this is more readable than the original: public UsersController : Controller { private const int PageSize = 25; public ActionResult Index(int page = 1, string sort = "", UserSearchViewModel search) ...


15

I would first suggest logging the database operation to see what SQL is executed. EF may not be running the SQL you think it is. The red flag I see is that you call ToList before the Skip and Take. Generally in LINQ statements, ToList forces execution immediately. This suggests your EF code is pulling every record from the table up front, building a list,...


15

There are clear guidelines for Enum Design where one of them says: X DO NOT use an enum for open sets (such as the operating system version, names of your friends, etc.). and this public enum MobiusBillingLocation { None, UK, US, SGSingapore } looks like an open set. const values might be a better solution in this case. You won't have to convert ...


14

As you have already mentioned, DbContext is a UoW, and DbSet is a repository -- there is no need to reimplement those patterns, unless you're into ridiculously useless complexity. Entity Framework wraps all pending changes in a transaction for you already, so each DbContext in an application contains DbSets that are somehow related. Blogs and Posts are ...


13

Without understanding too much of your scenario, here are a few pointers: I'm not that familiar with Entity Framework, but a common interface for UoW usually have the following behaviors: public interface UnitOfWork : IDisposable { bool IsInTransaction { get; } bool IsDirty { get; } // Same as your MarkModified() void BeginTransaction(); void ...


12

I get what you mean by "reversed": As opposed to: Makes sense, at least to me - the way I see UoW/Repository pattern (everybody seems to have their own take at this one, eh?), Entity Framework's DbContext is a unit-of-work, and an IDbSet<TEntity> is a repository. Hence, I tend to agree with having unit-of-work depend on repositories and not the ...


11

General rules of thumb: In order to take advantage of connection pooling (and you should), database connections should be as short lived as possible. Create, use, then immediately destroy. Single instance objects should always be agile (defined as not holding system resources e.g. db connections, file handles, etc.). If you need a single instance object to ...


11

I leaked Entity Framework into my domain services, because [...] Fail. The only reason you would ever want to wrap EF with your own repository+unit-of-work implementation, would be to make an interface between EF and your code, possibly to enable swapping EF for something else at one point or another. By leaking it, you defeat the entire purpose of the ...


10

I keep seeing all these really complex solutions to the lack of enum support, but most make things tricky when taking into account that next entity framework release is probably going to have enum support. Here's my solution. A sample class, before our workaround: public class Widget { public int ID { get; set; } public Status Status { get; set; } }...


10

A few simple things before getting to the meat of the problem. Method variables start with lowercase letters. There are a number of cases where you are not consistent with this and it will confuse other people that read your code. For the same reason, you should be consistent with camelCase of underscore_separated variable names. Don't create a new List ...


10

Don't use underscores in identifiers. If that's how the database has them, use a ColumnAttribute to specify the column name scratch that, you're going code-first. This: public int Tournament_Id { get; set; } public int Match_Id { get; set; } public int Team_Id { get; set; } public int PlayerAccount_Id { get; set; } Is much more seesharpesque like this: ...


10

The code itself is generally pretty clean looking, but I think it suffers in a few areas: InsertOrUpdate screams: "one method that is concerned with two distinct things". I see InsertOrUpdate as an anti-pattern. Coupling between Console/Output writing and persistence logic is too tight. The use of Func/Action seems unnecessary and overly complex. Their use ...


9

I made a small example to show you that all your worries are in the past. Before you implement this yourself though, keep Mat's remarks in mind: I made this as a quick sketch and separating the mappings from the context, naming conventions, etc are important for your code's clarity. That being said, this small setup shows you how you can change it: public ...


9

As Phil Sandler said, the code here is so thin that it's possibly questionable how much value unit tests actually add. However, I would say that their two main benefits are both still relevant: Living documentation of what a unit of code should do Instant feedback for future refactorings While it's much more likely that you'll add to PostsRepository rather ...


8

Your code is very difficult to read. (To give you an idea of how difficult it is, I've been looking at this for the past 2 hours so far) You are using traditional for loops when you instead should be using foreach loops. Your "loop variables" are poorly named. Look toward the end of the code. What is a c? What is a d? Leave the shorter names within ...


8

Your code seems really good, I would change the ITestingRepository methods to return IEnumerable<> instead of List<>. Also, I suggest you would add a method signature to your generic repository interface that would let you query your DbContext. public interface IGenericRepository<T> where T : class { T Get(int id); void Create(T ...


8

String literals vs. constants vs. enums I think that the biggest terrible thing that you are doing here is 1) using magic string literals that are not defined as constants. 2) using a string when you can use an enum instead. public enum SaveResult { Exists, NotSaved, Success } In fact, your current code returns "NotSaved", while you check for case "...


8

You are calling queries in a for-loop for each day. Database queries are slow compared to C# code execution. Send only one query to the DB for the whole range of days. You can do the grouping by day either as DB query or query the ungrouped records and group using LINQ-to-Objects later. Also you are querying the lstSalesRepIDs within the loop. Why? The ...


8

Very nice and comprehensive. Your names are clear and short, your code is obvious in it's intent. I just see two things that startle me a bit: if (context == null) { return new Entities(); } else return context; what you do here is ... amusing. This code doesn't do what you think it does ;) Both blocks are single line-statements, yet you put the ...


8

Yes, you need a IOC container to maintain your instances. No, your Unit of Work should not create them. With an IOC Container, you centralize the control over the objects and their lifetime. This is extremely powerful. Never should you have an instantiation of unit of work, repositories, ... throughout your application code. You're not the first person ...


8

Do I need ninject when implementing DAL with generic repository and unit of work? That's not the question. Inversion of control, repository and unit of work are patterns. Ninject is an IoC container - a tool that resolves dependencies and controls instantiation and object lifetime. You need Ninject (excellent choice, but many other IoC containers are ...


8

You're doing this for each record: var projectID = db.ProjectCodes.Where(project => project.Code == taskCode.Item4).Select(project => project.ID).FirstOrDefault(); That alone is a massive performance issue. Instead, before looping through taskCodes: extract the distinct Item4 (what a bad name, BTW) from your taskCodes, use those to retrieve the ...


8

Since you're not making any changes to existing objects, you might set the AutoDetectChangesEnabledproperty of your context to false. From MSDN: Gets or sets a value indicating whether the DetectChanges method is called automatically by methods of DbContext and related classes. The default value is true. Calling the Add calls the DetectChanges method ...


Only top voted, non community-wiki answers of a minimum length are eligible