1
\$\begingroup\$

Me colleague and I recently had a discussion on good practices in EF. So I showed one of mine.

He said it is a little bit muddler.

My practice consists of modifying the autogenerated class in a specific way. This is my starting model:

namespace PCServer.Data
{
    using System;
    using System.Collections.Generic;

    public partial class Post: IEntity
    {
        public int Id { get; set; }
        public string Title { get; set; }
        public string Message { get; set; }
        public DateTime Date { get; set; }

        public virtual Post ParentPost { get; set; }
        public virtual AspNetUser Author { get; set; }
    }
}

I like to extend it in this way:

public partial class Post
    {
        //This class "do" something, like adding a post or deleting a post
        public static class Do
        {
            public static void AddPost(ref ApplicationDbContext context, string postMessage)
            {
                //Create a post
                Post p = new Post();
                p.Title = "This is an example!";
                p.message = postMessage;
                p.Date = DateTime.UtcNow;

                //Adding to context
                BaseService.Add(post, out context);
            }

            public static void DeletePost(ref ApplicationDbContext context, int postId)
            {
                PostRepository postRepo = new PostRepository(context);

                postRepo.GetById(postId);

                //Removing from context
                BaseService.Remove(post, out context);
            }
        }

        //This class "Get" something, like all posts
        public static class Get
        {
            public static void GetPosts()
            {
                using(ApplicationDbContext context = new ApplicationDbContext())
                {
                    PostRepository postRepo = new PostRepository(context);
                    return postRepo.GetAllPosts();
                }
            }
        }

        //This class "Set" something, like title of the post or the post itself maybe
        public static class Set
        {
            public static void Title(ref ApplicationDbContext context, int postId, string title)
            {
                PostRepository postRepo = new PostRepository(context);
                Post post = postRepo.GetById(postId);
                post.Title = title;

                BaseService.Update(post, out context);
            }

            public static void ChangePost(ref ApplicationDbContext context, int postId, Post post)
            {
                PostRepository postRepo = new PostRepository(context);
                Post dbPost = postRepo.GetById(postId);
                dbPost = post;

                BaseService.Update(dbPost, out context);
            }
        }
    }

So, when I must do something with an Entity, I can (for example only):

ApplicationDbContext c = new ApplicationDbContext();

Post.Do.AddPost(ref c,"Hi!");
IEnumerable<Post> posts = Post.Get.GetPosts();

Post.Set.Title(ref c,100,"Changing title!");

And after all:

await BaseService.CommitAsync<Post>(c);

What do you think? Would you use it? Why?

Here is my original post of this on SO.

\$\endgroup\$
  • \$\begingroup\$ Same comment: don't do this. Move those methods to services. For one: what if you want to do stuff involving object graphs? Your methods are too granular. Also, this out context is pretty dirty code. It changes the context parameter entering ChangePost etc. while the consumer of these methods don't expect that. \$\endgroup\$ – Gert Arnold Nov 18 '17 at 23:10
  • \$\begingroup\$ @GertArnold Maybe i need to read some documentation on "out" and "ref", and maybe some on Entity Framework. in "BaseService.Add" i create the context if there is no one assigned and after i simply: context.Add(Entity) and return context \$\endgroup\$ – Met Nov 20 '17 at 10:32
2
\$\begingroup\$

The best aspect of your approach is that you keep the autogenerated class as is, which makes it more readable and clean, and keeps your extra code safe from deletion if you modify the database schema.

You can extend this class with a partial class to add a constructor setting automatic properties like Date to DateTime.UtcNow.

The static classes Do, Get, and Set are not using any of the members of the entity class, so I see no reason of being part of an entity unless you mean to add some Fluent Interface to your code. But this has pros and cons, and these static classes are not the best way to do so. Also, would you do that for all entities in your model? too much repetition, complexity, error prone, and dependency on context.

In Entity Framework, the best practices is to keep the entities unaware of their context or repository for a variety of reasons like single responsibility, and testabilty.

By using EF designer, you automatically generate code like below, which you can modify or extend

public class BloggingContext : ApplicationDbContext
{
    public System.Data.Entity.DbSet<PCServer.Data.Post> Posts { get; set; }
    public System.Data.Entity.DbSet<PCServer.Data.User> Users { get; set; }
    ...
}

You can create multiple context classes, but usually I did not see any benefit of doing so. One context for all your entities is fine.

The context owns the repository here as Posts property here is your repository, no need to create a new one. And DbSet has out-of-the-box all the repository methods you need like Create, Add, Find, and Remove.

Setting entity properties and calling repository methods would then be simple and called directly as you did in the last lines in your question, like below:

BloggingContext context = new BloggingContext();

context.Posts.Add(new Post { Title = "Hi!" });
List<Post> posts = await context.Posts.ToListAsync();

Post post = context.Posts.First(p => p.Id == 100); // or context.Posts.Find(100) if Post.Id is marked as [Key]  
post.Title = "Changing title!";

And after all:

await context.SaveChangesAsync();
\$\endgroup\$
  • \$\begingroup\$ I am trying to write less code possible without losing the meaning. I use to update the context inside the method and return it for commit or another operation. Usually i use a Repository, so i don't touch ever the context directly to avoid errors. For the moment i used a Service to do so (Api call Service that call Repository that return data to Service that return data to Api). So my question is, what approach is the best? \$\endgroup\$ – Met Nov 20 '17 at 10:29
  • \$\begingroup\$ API calling service is fine, but repository should not return data, it should contain data and you query it from the service. If you use {context.Posts}, you will be using a repository implemented for you {DbSet}, and don't worry, you won't be touching the context directly. Usually errors arise when you add extra code to do the same job of the standard library. \$\endgroup\$ – modeeb Nov 21 '17 at 11:01
  • \$\begingroup\$ I recommend that you create a sample ASP.NET MVC or Web API project using Visual Studio template and check how the template is using EF, controller, model, context, and DbSet. This is the best approach in my opinion. \$\endgroup\$ – modeeb Nov 21 '17 at 11:07
  • \$\begingroup\$ what do you mean with "it should contain data and you query it from the service"? \$\endgroup\$ – Met Nov 21 '17 at 12:35
  • \$\begingroup\$ In your approach, your {Service} calls {Do}, {Set} and {Get} which calls {Repository} and {Repository} uses {Context} to return data back. I would rather refactor it so {Service} directly uses {Context.Repository} as a collection of data and state (added, changed, deleted or pristine), which is already implemented in {DbSet} \$\endgroup\$ – modeeb Nov 22 '17 at 6:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.