You should move that up one layer by abstracting it out as well and providing a custom DbContext for testing purposes or by mocking it out. More information on that here here.
One thing that comes to mind is the long-lived DbContext object. Some say it isn't an exact necessity, others say you definitely should avoid it you definitely should avoid it but generally accepted is the notion that you should dispose of your DbContext. A unit of work corresponds to one method inside your repository to wrap a using statement around it.
You should move that up one layer by abstracting it out as well and providing a custom DbContext for testing purposes or by mocking it out. More information on that here.
One thing that comes to mind is the long-lived DbContext object. Some say it isn't an exact necessity, others say you definitely should avoid it but generally accepted is the notion that you should dispose of your DbContext. A unit of work corresponds to one method inside your repository to wrap a using statement around it.
You should move that up one layer by abstracting it out as well and providing a custom DbContext for testing purposes or by mocking it out. More information on that here.
One thing that comes to mind is the long-lived DbContext object. Some say it isn't an exact necessity, others say you definitely should avoid it but generally accepted is the notion that you should dispose of your DbContext. A unit of work corresponds to one method inside your repository to wrap a using statement around it.
Testability
Your repository implements an interface which will allow it to get stubbed out easily, so that's a very testable thing. However if you want to also (unit-test) your repositories themselves then you are stuck because you have a hardcoded dependency on DatabaseContext.
You should move that up one layer by abstracting it out as well and providing a custom DbContext for testing purposes or by mocking it out. More information on that here.
I have read countless opinions about what a repository should do. Are there any pragmatic reasons why my implementation might be bad?
One thing that comes to mind is the long-lived DbContext object. Some say it isn't an exact necessity, others say you definitely should avoid it but generally accepted is the notion that you should dispose of your DbContext. A unit of work corresponds to one method inside your repository to wrap a using statement around it.
Note that this would interfere with testing the repository itself so it's worth considering using integration-tests to test your repository in particular.
The
PostsRepositoryneeds to access theTagsdatabase set. Am I allowing this in the correct way? Know that I plan to implement aTagsRepositoryin the future.
I am not familiar enough with EF to answer that.
I throw an exception when the slug (which must be unique) is occupied. Should I return a
boolto indicate failure instead? Would this not violate the command-query segregation principle?
It does not violate it because it is just the result of your command, it is not the result of a query. It holds exactly the same value as an exception.
There is a more interesting way though: create a "result"-object. This can be as simple as
class CallResult {
bool Success { get; set; }
string Message { get; set; }
}
Which will be more descriptive than bool since you can also send a message along. Whether you choose this or an exception depends on your own preference: using an exception will force you to have a try-catch around it somewhere which is rather ugly but then again it will prevent you from accidentally leaving out validation on the return type (either a try-catch or if(result.Success)) which is not enforced with a custom type.
I am aware that the
Editmethod is hard to reason about and I am working on that. It is for this reason that my code does not currently adhere to DRY.
Since I don't see an Edit method I'll assume you haven't written it yet. DRY is nice and all but for some niche situations (CRUD actions, tests, ..) I would argue that readability is more important than having a few lines similar to eachother.
I suppose you could create a situation like this, but that depends on how different each action is.
void Process(Post post) {
post.Slug = SlugConverter.Convert(post.Slug);
if (context.Posts.Find(post.Slug) != null)
{
throw new Exception("tag already exists. choose another.");
}
post.Summary = Summarize(post.Content);
AttachTags(post);
}
void Create(Post post) {
Process(post);
post.PublishedAt = DateTime.Now;
context.Posts.Add(post);
context.SaveChanges();
}
void Update(Post post) {
Process(post);
context.Posts.Add(post);
context.SaveChanges();
}
Note how I performed the validation before doing the other work.
A method should be written as [action][context]. AllPublishedPosts makes me think it's a property, not a method. I would change this to GetAllPublishedPosts. I'm not speaking out about All since that's a special situation, I suppose.