CRUD – Destroyer of Worlds

As web developers a large part of what we do is CRUD.  The average Service class in my project is at least 50% CRUD methods, and often a lot more.

If you’re anything like me, you’ve probably not given much thought to CRUD methods, you just write them as required.  However, I recently had a small epiphany, this approach to CRUD was making the code base sprawl, approaches to CRUD from the team were non-standard.  Which was leading to new CRUD methods being created, when not required, because of the difficulty of understanding what was there.

I wouldn’t start from here…

So how did it come to this?  I use the Repository pattern for my DB layer and for whatever reason a lot of the documentation around this pattern have method signatures like this:

Customer GetCustomerById(int customerId)
Customer GetCustomerByAddress(Address address)

Thinking that this was the accepted pattern I just blindly started adding this style of methods into my code base, not only in the Repository but in the Service layer too.

Here’s an interface from out codebase for working with images, were you can see CRUD methods jumbled in with other methods to get PagedLists etc.  All in all not very obvious.

public interface IMediaItemsService
{
    PagedList<MediaItem> GetForUserId(int userId, MediaTypeEnum mediaType, int page, int pageSize);
    List<MediaItem> GetForUsername(string username, MediaTypeEnum? mediaType, int? numRecords);
    void Save(MediaItem mediaItem);
    void Delete(MediaItem mediaItem);
    void Delete(int mediaItemId);
    void Delete(int mediaItemId, int currentUserId);
    MediaItem GetByMediaItemId(int mediaItemId);
    MediaItem GetByUniqueId(Guid uniqueId);
    IPagedList<MediaItem> GetAll(int page, int pageSize);
    IPagedList<MediaItem> Search(MediaSearchParams mediaSearchParams, int page, int pageSize);
    MediaItem Update(MediaForm mediaModel);
    IEnumerable<MediaItem> GetAllMediaItemsForRequest(Guid uploadRequestId);
}

The Anti-Pattern

What I’ve come to realise is the Byxxxx is an anti-pattern and makes your interfaces far harder to work with.

Not only that but we had some non-standard approaches to Creates, sometimes called Save (does this do updates), Delete maybe called Remove etc.  The upshot being that when approaching adding some functionality you had to study the Service class to separate the CRUD methods from other methods.  It was at this point I realised there was a much better approach to dealing with CRUD (credit must go to @leegunn for planting the germ of the idea).

Convention over Configuration, Overloading and Areas

In my opinion, the most important message from Convention over Configuration (taken from the Wikipedia article) is:

“… a software design paradigm which seeks to decrease the number of decisions that developers need to make…”

So I came up with the following CRUD conventions for our project

  1. CRUD Methods will be added at the top of each method in a Region called CRUD
  2. All CRUD methods should be overloaded where-ever possible
  3. Overloads should ideally call down to one method containing all the business logic – (hopefully I’ll manage to expand on this in an additional post).
  4. CRUD methods should be entered in the order Create, Read, Update, Delete.
  5. Create Methods will be named Create, and should always return the object that was created
  6. Read Methods will be named Retrieve where they return a single object
  7. Read Methods will be named  RetrieveAll  where they return an IEnumerable<T> collection of objects
  8. Update Methods will be called Update and should always return the object that was updated
  9. Delete methods will be called Delete and return void, an exception may be thrown if an error occurs during the transaction.

The Results

By applying these rules to the interface above we get:

public interface IMediaItemsService
{
#region CRUD

MediaItem Create(MediaItem mediaItem);
MediaItem Retrieve(int mediaItemId);
MediaItem Retrieve(Guid uniqueId);
IEnumerable<MediaItem> RetrieveAll(Guid uploadRequestId);
IEnumerable<MediaItem> RetrieveAll (string username, int? numRecords);
MediaItem Update(MediaForm mediaModel);
MediaItem Update(MediaItem mediaItem);
void Delete(MediaItem mediaItem, int currentUserId);
void Delete(int mediaItemId, int currentUserId);
void Delete(MediaItem mediaItem, int currentUserId, bool isAdmin);
void Delete(int mediaItemId, int currentUserId, bool isAdmin);

#endregion

IPagedList<MediaItem> GetAll(int page, int pageSize);
PagedList<MediaItem> GetForUserId(int userId, MediaTypeEnum mediaType, int page, int pageSize);
IPagedList<MediaItem> Search(MediaSearchParams mediaSearchParams, int page, int pageSize);
}

By leveraging overloading where-ever possible we’ve got rid of Byxxx, which makes working with Reads a lot easier.  I can just do MediaItemsService.Retrieve and study the available overloads.  Same for Updates, Deletes etc.

Additionally by separating your interfaces into CRUD and Non-CRUD methods, it makes it easier to spot when a new class, factory etc might be appropriate.  In our case we can see that the non CRUD methods could do with being refactored, possibly rename to RetrieveAllPaged etc.  The point being the refactoring is now easy to spot.

So by conforming to CoC principles for CRUD methods, we’ve definitely decreased the number of decisions our devs needs to make.  Score! 🙂

Postscript

While writing this post I came across an intriguing idea mentioned at the bottom of this Stackoverflow question about using lambda expressions within your CRUD interface, to allow you to effectively pass a query through to the DB.  This would drastically cut down on overloads not to mention repository methods.

So interface would be

T Single<T>(Expression<Func<T, bool>> expression) where T : class, new();

Usage would be

var user = _dbService.Single<User>(user => user.ID == 12);

There are a few things that could make this tricky to work with, especially if you needed to enforce business rules etc.  However, as a starting place for working with CRUD it’s a really smart idea, and warrants more investigation.

Update 07/12/11

After comments from @alanjmburns I decided he was right and the Delete methods are better to return void rather than a status, and throw an exception if necessary.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s