Code First Entity Framework

Is the following good design for doing entity framework code first? What am I missing for a production system? I haven't included all my code, just a snapshot...

My application, doesn't update the database; it just reads it.

```
public class Document
{
[Key, ScaffoldColumn(false)]
public Int32 DocumentID { get; set; }

[ScaffoldColumn(false)]
public Boolean? Status { get; set; }

[Display(Name = "Document Text"), DataType(DataType.MultilineText)]
public String DocumentText { get; set; }

[ScaffoldColumn(false), DataType(DataType.Url)]
public String DocumentFolderPath { get; set; }

[ScaffoldColumn(false), DataType(DataType.Url)]
public String DocumentJSONPath { get; set; }

[ForeignKey("DocumentID")]
public virtual DocumentMetadata Metadata { get; set; }

[ForeignKey("DocumentID")]
public virtual ICollection Pages { get; set; }
}

public class Page
{
[Key, ScaffoldColumn(false)]
public Int32 PageID { get; set; }

[Required, Display(Name = "Page"), DataType(DataType.Text)]
public Int32 PageNumber { get; set; }

[Required, ScaffoldColumn(false)]
public Int32 DocumentID { get; set; }

[ScaffoldColumn(false), DataType(DataType.ImageUrl)]
public String ImagePath { get; set; }

[Required, Display(Name = "Page Text"), DataType(DataType.MultilineText)]
public String PageText { get; set; }

[NotMapped]
public String HighlightedText { get; set; }

[ForeignKey("PageID")]
public virtual ICollection Words { get; set; }
}

public class ArchiveDatabaseInitializer : DropCreateDatabaseIfModelChanges
{
protected override void Seed(ArchiveContext context)
{
try
{
GetDocuments().ForEach(d => context.Documents.Add(d));
GetDocumentMetatdata().ForEach(d => context.DocMetadata.Add(d));
GetPages().ForEach(d => context.Pages.Add(d));
GetWords().ForEach(d => context.Words.Add(d));
context.SaveCha

If you chose the code-first approach, I presume you don't have to deal with the constraints of an existing database. If that assumption is correct, then I'd say you've done it the hard way.

Convention Over Configuration

Entity Framework can infer keys (primary and foreign) from the names of your entity members; your code isn't leveraging this formidable capability.

For a greenfield project, I like to start with a base entity type:

public abstract class EntityBase
{
    public int Id { get; set; } // inferred PK
    public DateTime DateInserted { get; set; }
    public DateTime? DateUpdated { get; set; }
}

Now I can derive, say, Document from that class:

public class Document : EntityBase
{
    public bool? Status { get; set; }
    public string DocumentText { get; set; }
    // ...

    public virtual DocumentMetadata Metadata { get; set; } // inferred FK
}

The PK is inferred from the inherited Id property (or [TypeName]Id), and the mere mention of a virtual property referencing another entity type is enough for EF to understand you want a FK there.

I'll pause here to mention that I was somewhat thrown off by your non-usage of the C# language aliases for System.String, System.Int32 and System.Boolean. They're typically written as string, int and bool.

You mention you're only reading from the database. This sounds like you're going code-first with an existing database. It's certainly possible, but the best way to do this in my opinion, is to reverse-engineer the database into entity classes - then you get the best of both worlds, and the generated code can teach you a lot about how EF works.

I think the presence of DisplayAttribute and NotMappedAttribute smells like you're possibly using your entities as both data entities and presentation / ViewModels. I think I would separate these concerns and create separate classes for presentation purposes, where HighlightedText means something relevant.

Also note that relying on a declarative DisplayAttribute will make it quite a pain to localize the application, if you ever need to do that in the future; you'll want these things resolved at run-time, accounting for Thread.CurrentThread.CurrentUICulture.

The naming is overall ok, except I don't like the Status column. It's just too vague of a name. What do the null, true and false values mean? The name should convey that meaning. Also I wouldn't prefix most of Document's members with the word "Document", and ID should follow the PascalCasing convention and be Id.

StackExchange Code Review Q#40381, answer score: 4