4
\$\begingroup\$

I have one entity that depends on another however in my API service class I would like to just deal with the dependent entity. Am I doing this correctly and is there a better way to go about this?

Post Entity

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Web;

namespace API.Models
{
    public class Post
    {
        [Key]
        public int Id { get; set; }
        public string Author { get; set; }
        public int Type { get; set; }
        public int UpVotes { get; set; }
        public int DownVotes { get; set; }
        public int Comments { get; set; }
        public int Shares { get; set; }
        public int Views { get; set; }
        public int Favorites { get; set; }
        public bool Flagged { get; set; }
        public bool Locked { get; set; }
        public int ContentVersion { get; set; }
        public DateTime Created { get; set; }
        public DateTime Updated { get; set; }
    }
}

PostContent Entity

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;

namespace API.Models
{
    public class PostContent
    {
        public int Id { get; set; }
        public int PostId { get; set; }
        public int Version { get; set; }
        public string Title { get; set; }
        public string Body { get; set; }
        public string Author { get; set; }
        public DateTime Updated { get; set; }
        public virtual Post Post { get; set; }
    }
}

PostService Class

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using API.Models;
using System.Data.Entity;

namespace API.Services
{
    public interface IPostService
    {
        IEnumerable<PostContent> GetAll();
        IEnumerable<PostContent> GetAllVersions(int id);
        PostContent Get(int id);
        PostContent Add(PostContent content, int typeId);
        bool Update(PostContent content);
        void Delete(int id);
    }

    public class PostService : IPostService
    {
        public IEnumerable<PostContent> GetAll()
        {
            using (var db = new ApplicationDbContext())
            {
                return db.PostContents.Where(p => p.Post.Locked == false).ToList();
            }
        }

        public IEnumerable<PostContent> GetAllVersions(int id)
        {
            return GetAll().Where(p => p.Post.Id == id).ToList();
        }

        public PostContent Get(int id)
        {
            return GetAllVersions(id).SingleOrDefault(p => p.Post.ContentVersion == p.Version);
        }

        public PostContent Add(PostContent content, int typeId)
        {
            var now = DateTime.UtcNow;
            if (content == null)
                throw new ArgumentNullException("Content");
            var post = new Post()
            {
                Author = content.Author,
                Type = typeId,
                UpVotes = 0,
                DownVotes = 0,
                Comments = 0,
                Shares = 0,
                Views = 0,
                Favorites = 0,
                Flagged = false,
                Locked = false,
                ContentVersion = 1,
                Created = now,
                Updated = now
            };
            using (var db = new ApplicationDbContext())
            {
                db.Posts.Add(post);
                content.PostId = post.Id;
                content.Version = 1;
                content.Updated = now;
                db.PostContents.Add(content);
                db.SaveChanges();
            }
            return content;
        }

        public bool Update(PostContent content)
        {
            var now = DateTime.UtcNow;
            if (content == null)
                throw new ArgumentNullException("Content");
            if (Get(content.PostId) == null)
                return false;
            var _content = Get(content.PostId);
            var post = _content.Post;
            content.Version = ++_content.Version;
            post.ContentVersion = content.Version;
            post.Updated = now;
            content.Updated = now;
            using (var db = new ApplicationDbContext())
            {
                db.Entry(post).State = EntityState.Modified;
                db.PostContents.Add(content);
                db.SaveChanges();
                return true;
            }
        }

        public void Delete(int id)
        {
            var post = Get(id).Post;
            if (post == null)
                return;
            post.Locked = true;
            using (var db = new ApplicationDbContext())
            {
                db.Entry(post).State = EntityState.Modified;
                db.SaveChanges();
            }
        }
    }
}
\$\endgroup\$
3
\$\begingroup\$

Some issues I noticed:

  1. GetAllVersions(int id) - it's absolutely unclear what the id refers to without reading the code. This should be renamed to GetAllVersionsForPost(int postId). Same goes for Get - should be GetCurrentContentForPost(int postId)

  2. In order to get all the contents associated with a specific Post you load all posts into memory and then filter for the single post id you are interested in. This happens in GetAllVersions and Get. This is very wasteful and will come to bite you if the amount of posts grows larger. You should do the filtering inside the database context so that it happens on the database service side and not in your application layer.

  3. You are probably aware of this but if the Update method gets called with a content for the same Post from two different threads (i.e. maybe via simultaneous web requests) then you will end up in an inconsistent state. Not sure if that can actually happen in your overall application flow but should be kept in mind.

\$\endgroup\$
  • \$\begingroup\$ thanks for the awesome suggestions. I will integrate alot of these today. However I have some concerns with just how the api will pass some of this stuff. There are pieces of the model that I don't really want to pass through the api and I am trying to resolve how to handle that. I am aware of the Update issue, and I was planning on handling that in the controller itself, would you recommend handling it in the Service class instead? \$\endgroup\$ – aaronmallen Sep 26 '14 at 15:33
  • \$\begingroup\$ take a look at some of the changes I've made here let me know what you think. \$\endgroup\$ – aaronmallen Sep 28 '14 at 4:39

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.