HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Post Service Class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
postserviceclass

Problem

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 GetAll();
IEnumerable GetAllVersions(int id);
PostContent Get(int id);
PostContent Add(PostContent content, int typeId);
bool Update(PostContent content);
void Delete(int id);
}

publi

Solution

Some issues I noticed:

-
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)

-
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.

-
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.

Context

StackExchange Code Review Q#63810, answer score: 3

Revisions (0)

No revisions yet.