patterncsharpMinor
Code-First Notes Database
Viewed 0 times
codedatabasefirstnotes
Problem
I need to learn more about Entity Framework, so I created a database to store notes. The database definition is as follows:
I wrote the following helper functions (I'm working off a Console Application for now):
```
class Program
{
private static void AddUser(string userName)
{
using (var db = new NotesContext())
{
db.Users.Add(new User { UserName = userName });
db.SaveChanges();
}
}
private static void RemoveUser(int userId)
{
using (var db = new NotesContext())
{
var user = db.Users.FirstOrDefault(u => u.UserId == userId);
if (user != null)
{
db.Users.Remove(user);
db.SaveChanges();
}
}
}
private static void ClearUsers()
{
using (var db = new NotesContext())
{
db.Users.RemoveRange(db.Users);
db.SaveChanges();
}
}
private static void AddNote(int userId, string title, string content)
{
using (var db = new NotesContext())
{
db.Notes.Add(new Note { UserId = userId, Title = title, Content = content });
db.SaveChanges();
}
}
private static void RemoveNote(int noteId)
{
using (var db = new NotesContext())
{
var note = db.Notes.FirstOrDefault(u => u.NoteId == noteId);
public class User
{
[Key]
public int UserId { get; set; }
[MaxLength(20)]
public string UserName { get; set; }
public virtual List Notes { get; set; }
}
public class Note
{
[Key]
public int NoteId { get; set; }
[MaxLength(30)]
public string Title { get; set; }
[MaxLength(200)]
public string Content { get; set; }
public int UserId { get; set; }
public virtual User User { get; set; }
}
public class NotesContext : DbContext
{
public DbSet Users { get; set; }
public DbSet Notes { get; set; }
}I wrote the following helper functions (I'm working off a Console Application for now):
```
class Program
{
private static void AddUser(string userName)
{
using (var db = new NotesContext())
{
db.Users.Add(new User { UserName = userName });
db.SaveChanges();
}
}
private static void RemoveUser(int userId)
{
using (var db = new NotesContext())
{
var user = db.Users.FirstOrDefault(u => u.UserId == userId);
if (user != null)
{
db.Users.Remove(user);
db.SaveChanges();
}
}
}
private static void ClearUsers()
{
using (var db = new NotesContext())
{
db.Users.RemoveRange(db.Users);
db.SaveChanges();
}
}
private static void AddNote(int userId, string title, string content)
{
using (var db = new NotesContext())
{
db.Notes.Add(new Note { UserId = userId, Title = title, Content = content });
db.SaveChanges();
}
}
private static void RemoveNote(int noteId)
{
using (var db = new NotesContext())
{
var note = db.Notes.FirstOrDefault(u => u.NoteId == noteId);
Solution
Foreign Key
Your foreign key is typed to
Additionally, you can take advantage of EF Code First Conventions to not require the foreign key id fields or primary key id fields at all, and have EF imply them for you. This will lead to your entities most closely resembling your domain model, instead of DTOs.
Structure
There isn't a ton wrong with your code being structured as it is. Anything extra would really be redundancy at this stage, but then it isn't very large at this stage.
The following are recommendations that don't make sense in a trivial project like this, but once you start involving substantial logic, they will be a huge benefit.
Put your logic in a class that makes sense
I like to use a Service Object for this.
That DbContext Issue
The approach of using a whole db context per operation would be an efficiency issue but you appear to be aware of that.
However, performance aside, I'd recommend against this for a number of reasons.
These methods creating their own db contexts is a violation of the Single Responsibility Principle. It shouldn't matter what DB store you've got in there or when it's saved. Leave that to the consumer of those methods. On top of that, you cannot swap out what DbContext is used in the case of testing.
Secondly the static methods are permanently affecting state. It is very difficult to test these methods without first creating an entire database.
To fix this, you should inject this
Lastly, there's a lot of duplication in the boilerplate of creating a DbContext, so we can extract that out to a single method. With it in a single method, it frees you to be more open to altering db code too, allowing you to implement transactions, for example, in the future.
Don't pass ids around, pass the entities
Now that Program is acting as a barrier between the nasty, stringly-typed command line frontier and the beautiful, pure objects domain, let's make that actual domain use pure objects.
You've got these lovely types for a reason, benefit from the type checker and get extra correctness by passing the whole entity between your functions.
Recommendation
(assuming the EF model recommendation earlier)
```
class NoteService
{
readonly NotesContext db;
public NoteService(NotesContext context)
{
db = context;
}
public Note CreateNote(User user, string title, string content) => new Note { User = user, Title = title, Content = content };
public void AddNote(Note note) => db.Notes.Add(note);
public Note GetNote(int id) => db.Notes.Find(id);
public void RemoveNote(Note note) => db.Notes.Remove(note);
public void ClearNotes() => db.Notes.RemoveRange(db.Notes);
}
class UserService
{
readonly NotesContext db;
public UserService(NotesContext context)
{
db = context;
}
public User CreateUser(string userName) => new User() { UserName = userName };
public void AddUser(User user) => db.Users.Add(user);
public User GetUser(int id) => db.Users.Find(id);
public void RemoveUser(User user) => db.Users.Remove(user);
public void ClearUsers() => db.Users.RemoveRange(db.Users);
}
class Program
{
private static void ExecuteAgainstDatabase(Action action)
{
using (var db = new NotesContext())
{
action(db);
db.SaveChanges();
}
}
private static void ExecuteForUsers(Action action) => ExecuteAgainstDatabase(db => action(new UserService(db)));
private static void ExecuteForNotes(Action action) => ExecuteAgainstDatabase(db => action(new NoteService(db)));
private static void AddUser(string userName) => ExecuteForUsers(us => us.AddUser(us.CreateUser(userName)));
private static void RemoveUser(int userId) => ExecuteForUsers(us => us.RemoveUser(us.GetUser(userId)));
private static void ClearUsers() => ExecuteForUsers(us => us.ClearUsers());
private static void AddNote(int
Your foreign key is typed to
List. I recommend using a HashSet to prevent adding the same entities twice to you navigation properties. I also recommend instantiating it, too, so that you don't get null reference exceptions on new Users that haven't been loaded from EF.public virtual ICollection Notes { get; set; } = new HashSet()Additionally, you can take advantage of EF Code First Conventions to not require the foreign key id fields or primary key id fields at all, and have EF imply them for you. This will lead to your entities most closely resembling your domain model, instead of DTOs.
public class User
{
[MaxLength(20)]
public string UserName { get; set; }
public virtual ICollection Notes { get; set; } = new HashSet();
}
public class Note
{
[MaxLength(30)]
public string Title { get; set; }
[MaxLength(200)]
public string Content { get; set; }
public virtual User User { get; set; }
}
public class NotesContext : DbContext
{
public DbSet Users { get; set; }
public DbSet Notes { get; set; }
}Structure
There isn't a ton wrong with your code being structured as it is. Anything extra would really be redundancy at this stage, but then it isn't very large at this stage.
The following are recommendations that don't make sense in a trivial project like this, but once you start involving substantial logic, they will be a huge benefit.
Put your logic in a class that makes sense
Program is doing too much, move your DB logic into a Notes or Users class. For command line apps, I prefer to use Program to bridge the chasm between the screaming maw of stringly-typed CMD world and the beautiful, shining crystal palaces of your OOP domain.I like to use a Service Object for this.
That DbContext Issue
The approach of using a whole db context per operation would be an efficiency issue but you appear to be aware of that.
However, performance aside, I'd recommend against this for a number of reasons.
These methods creating their own db contexts is a violation of the Single Responsibility Principle. It shouldn't matter what DB store you've got in there or when it's saved. Leave that to the consumer of those methods. On top of that, you cannot swap out what DbContext is used in the case of testing.
Secondly the static methods are permanently affecting state. It is very difficult to test these methods without first creating an entire database.
To fix this, you should inject this
DbContext dependency.Lastly, there's a lot of duplication in the boilerplate of creating a DbContext, so we can extract that out to a single method. With it in a single method, it frees you to be more open to altering db code too, allowing you to implement transactions, for example, in the future.
Don't pass ids around, pass the entities
Now that Program is acting as a barrier between the nasty, stringly-typed command line frontier and the beautiful, pure objects domain, let's make that actual domain use pure objects.
You've got these lovely types for a reason, benefit from the type checker and get extra correctness by passing the whole entity between your functions.
Recommendation
(assuming the EF model recommendation earlier)
```
class NoteService
{
readonly NotesContext db;
public NoteService(NotesContext context)
{
db = context;
}
public Note CreateNote(User user, string title, string content) => new Note { User = user, Title = title, Content = content };
public void AddNote(Note note) => db.Notes.Add(note);
public Note GetNote(int id) => db.Notes.Find(id);
public void RemoveNote(Note note) => db.Notes.Remove(note);
public void ClearNotes() => db.Notes.RemoveRange(db.Notes);
}
class UserService
{
readonly NotesContext db;
public UserService(NotesContext context)
{
db = context;
}
public User CreateUser(string userName) => new User() { UserName = userName };
public void AddUser(User user) => db.Users.Add(user);
public User GetUser(int id) => db.Users.Find(id);
public void RemoveUser(User user) => db.Users.Remove(user);
public void ClearUsers() => db.Users.RemoveRange(db.Users);
}
class Program
{
private static void ExecuteAgainstDatabase(Action action)
{
using (var db = new NotesContext())
{
action(db);
db.SaveChanges();
}
}
private static void ExecuteForUsers(Action action) => ExecuteAgainstDatabase(db => action(new UserService(db)));
private static void ExecuteForNotes(Action action) => ExecuteAgainstDatabase(db => action(new NoteService(db)));
private static void AddUser(string userName) => ExecuteForUsers(us => us.AddUser(us.CreateUser(userName)));
private static void RemoveUser(int userId) => ExecuteForUsers(us => us.RemoveUser(us.GetUser(userId)));
private static void ClearUsers() => ExecuteForUsers(us => us.ClearUsers());
private static void AddNote(int
Code Snippets
public class User
{
[MaxLength(20)]
public string UserName { get; set; }
public virtual ICollection<Note> Notes { get; set; } = new HashSet<Note>();
}
public class Note
{
[MaxLength(30)]
public string Title { get; set; }
[MaxLength(200)]
public string Content { get; set; }
public virtual User User { get; set; }
}
public class NotesContext : DbContext
{
public DbSet<User> Users { get; set; }
public DbSet<Note> Notes { get; set; }
}class NoteService
{
readonly NotesContext db;
public NoteService(NotesContext context)
{
db = context;
}
public Note CreateNote(User user, string title, string content) => new Note { User = user, Title = title, Content = content };
public void AddNote(Note note) => db.Notes.Add(note);
public Note GetNote(int id) => db.Notes.Find(id);
public void RemoveNote(Note note) => db.Notes.Remove(note);
public void ClearNotes() => db.Notes.RemoveRange(db.Notes);
}
class UserService
{
readonly NotesContext db;
public UserService(NotesContext context)
{
db = context;
}
public User CreateUser(string userName) => new User() { UserName = userName };
public void AddUser(User user) => db.Users.Add(user);
public User GetUser(int id) => db.Users.Find(id);
public void RemoveUser(User user) => db.Users.Remove(user);
public void ClearUsers() => db.Users.RemoveRange(db.Users);
}
class Program
{
private static void ExecuteAgainstDatabase(Action<NotesContext> action)
{
using (var db = new NotesContext())
{
action(db);
db.SaveChanges();
}
}
private static void ExecuteForUsers(Action<UserService> action) => ExecuteAgainstDatabase(db => action(new UserService(db)));
private static void ExecuteForNotes(Action<NoteService> action) => ExecuteAgainstDatabase(db => action(new NoteService(db)));
private static void AddUser(string userName) => ExecuteForUsers(us => us.AddUser(us.CreateUser(userName)));
private static void RemoveUser(int userId) => ExecuteForUsers(us => us.RemoveUser(us.GetUser(userId)));
private static void ClearUsers() => ExecuteForUsers(us => us.ClearUsers());
private static void AddNote(int userId, string title, string content)
{
ExecuteAgainstDatabase(db =>
{
var noteService = new NoteService(db);
var userService = new UserService(db);
var user = userService.GetUser(userId);
noteService.AddNote(noteService.CreateNote(user, title, context));
});
}
private static void RemoveNote(int noteId) => ExecuteForNotes(ns => ns.RemoveNote(ns.GetNote(noteId)));
private static void ClearNotes() => ExecuteForNotes(ns => ns.ClearNotes());
}Context
StackExchange Code Review Q#163019, answer score: 3
Revisions (0)
No revisions yet.