3
\$\begingroup\$

I would like to a review of my database design.

There are 3 tables + ASP.NET Identity tables.

The Users could have many postcards.

Here's my tables without Identity.

Postcard, Country, and Error tables

I attached also my scripts of creating tables.

CREATE TABLE [dbo].[Country](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [Name] [nvarchar](50) NOT NULL,
    [Code] [nchar](10) NOT NULL
) ON [PRIMARY]

CREATE TABLE [dbo].[Error](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [Message] [nvarchar](max) NULL,
    [Stacktrace] [nvarchar](max) NULL,
    [CreatedDate] [nvarchar](max) NULL
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]

CREATE TABLE [dbo].[Postcard](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    [CountryFromId] [int] NOT NULL,
    [CountryToId] [int] NOT NULL,
    [TravelTime] [int] NULL,
    [Confirmation] [bit] NOT NULL,
    [UserFromId] [int] NULL,
    [UserToId] [int] NOT NULL,
    [Status] [int] NULL,
    [SentDate] [datetime2](7) NOT NULL,
    [ReceivedDate] [datetime2](7) NULL,
    [ImageId] [int] NULL
) ON [PRIMARY]

Do you think is it good design, or shoud I change something? I know that proper databse design is key in good application architecture so I would like to have good database design.

\$\endgroup\$
1
\$\begingroup\$

Looking at each of the tables, there are a few design problems that stand out.

Country

Depending on the target countries and language, some country names could be longer than 50 characters. For example, "Separate Customs Territory of Taiwan, Penghu, Kinmen, and Matsu (Chinese Taipei)", the longest English country name, is 80 characters long, meaning you would get an error from the SQL database in [Name] [nvarchar](50) if you tried to insert this value.

For the [Code] [nchar](10) there is already some official lists of country codes (reference) which could only be a [nchar](2) field for ISO codes, or a [nchar](3) field for UN codes. Using a standard country code would also make it more standards-compliant.

Error

The [Message] [nvarchar](max) and [Stacktrace] [nvarchar](max) are problematic. You should avoid using the MAX types unless you actually need to store up to 2 GB of text in a field (or just over 1 billion 2-byte characters - source). See Using MAX text or more specific, smaller type from Database Administrators site. The reasoning is in large part because the MAX types are stored outside of the table, only a reference to the field is stored in the table, and as such they are optimized differently.

[CreatedDate] [nvarchar](max) should instead be a [datetime2] or similar type like you are using in other places, and not a character field. Never store dates and/or times in a text field (and especially not in a MAX text field), as it makes it much more difficult to manipulate the data.

The only exception to this is if you are storing/receiving data from a system where date/time values are stored in a non-standard text format like Wed 06-Jul-16 10:31 AM but even in that case it would be better to try to convert it to a compatible format before storing it in the database as a date/time field.

Postcard

This table contains mostly IDs from what looks to be other tables, however I don't see any foreign key references to the other tables, which defeats the primary purpose of have keys for rows in the first place. Here is some useful information on MSDN.

Also make sure to set a PRIMARY KEY constraint on the main ID field of each table. The syntax for keys is like this:

CREATE TABLE [dbo].[Postcard](
    [Id] [bigint] IDENTITY(1,1) NOT NULL,
    CONSTRAINT [PK_Postcard] PRIMARY KEY NONCLUSTERED ([Id]),
    [CountryFromId] [int] NOT NULL,
    CONSTRAINT [FK_Postcard_CountryFromId] FOREIGN KEY ([[CountryFromId])
        REFERENCES [dbo].[Country]([Id]),
    [CountryToId] [int] NOT NULL,
    CONSTRAINT [FK_Postcard_CountryToId] FOREIGN KEY ([[CountryToId])
        REFERENCES [dbo].[Country]([Id]),
    -- etc.

Using an 8-byte [bigint] type for your Postcard.Id seems like an overly large type. According to MSDN the range for [bigint] is "-2^63 (-9,223,372,036,854,775,808) to 2^63-1 (9,223,372,036,854,775,807)". If you really plan on having that many postcard records then fine, but otherwise just an [int] type would be OK.

For consistency your [Status] [int] should probably be instead named [StatusId].

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Regarding foreign keys, I'd name them the same across tables to make that association a bit more obvious. And in the long run this will make the SQL easier to write and read. \$\endgroup\$ – radarbob Jul 7 '16 at 12:01

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.