protected void gvBooks_RowCommand(object sender, GridViewCommandEventArgs e)
{
if (e.CommandName == "AddToCart")
{
long bookId = Convert.ToInt64(e.CommandArgument);
//ADD YOUR CODE HERE!!!
PopulateCartGrid();
}
}
One requirement was that the total for the cart had to be calculated, and persisted in the database.
There was also a second stage of the homework which required a discount to be added of £5 for each different book that was added, so only 1 book gets no discount, 2 of the same book gets no discount, 2 different books gets £5 discount, 3 different books get £10 discount etc.
Here are some of the proposed solutions, starting with the code in the UI and working down:
User Interface Code
Here are some examples of the submissions for code in the UI, showing how the UI interacts with the Application Layer:
Example 1
ICartService cartService = new CartService();
Cart cart = cartService.GetCurrentUsersCart();
IBookService bookService = new BookService();
Book book = bookService.GetById(bookId);
cart.AddBook(book);
cartService.Save(cart);
This code has three calls to the application layer to add the book to the cart, including a Save() service that takes an entity as a parameter (something that should be avoided). Also, here we have UI code calling a method on an entity, something which should also be avoided, and would be impossible if using DTOs or WCF.
Example 2
ICartService cartService = new CartService();
IBookService bookService = new BookService();
var book = bookService.GetById(bookId);
Cart cart = cartService.GetCurrentUsersCart();
cartService.AddItemtoCart(cart,book);
This is an improvement. There are still three calls to the UI, but no methods being called on the Cart entity, and there is no Save service which takes an entity as a parameter. The AddItemToCart service performs the logic associated with adding the item to the cart and then persists it, all in one shot. But the consumer of the service has to get the Cart and Book entities before calling AddItemToCart. This could be encapsulated further by passing the IDs only.
Example 3
ICartService cartService = new CartService();
cartService.AddBook(bookId);
This is all that is required. It is not even required to pass in the user name, because the Application Layer is running in the same process, and because the UI uses Windows authentication, the Application Layer can get the user from the principal identity. All that is needed is the bookId. The service can then load the book by the ID, the cart by the username, perform any business logic and then save the result.
Summary
All commands should be done in one service call. A command can be defined as anything that changes the state of the business, or anything where a user presses a 'Submit' button. The reason for this is that this may not be the only consumer of this service, there may be a mobile App that consumes it, or possibly another service. This example is trivial, but if the system starts accessing the file system, or interacting with other services, the code can become very comlpex. Any systems consuming the service would have to replicate all required code.
Application Layer Code
Example 1
public void AddBook(Cart cart, long bookId)
{
BookService bookService = new BookService();
Book selectedBook = bookService.GetById(bookId);
cart.Books.Add(selectedBook);
cart.Total = (from b in cart.Books select b.Price).Sum();
}
This submission is from before the addition of the extra requirement for discounts. This shows an example of a service call in which the Cart has to have been loaded externally, and which which the Cart has to be saved externally. Another problem here is that the business logic of calculating the total is contained in the Application Layer, not the Domain.
Example 2
public void AddItemtoCart(Cart cart, Book book)
{
ICartRepository cartRepository = new CartRepository();
cart.AddItem(book);
using (ITransactionProvider transactionProvider = new TransactionProvider())
{
transactionProvider.BeginTransaction();
cartRepository.SaveOrUpdate(cart);
transactionProvider.CommitTransaction();
}
}
This example is an improvement, because the business logic is encapsulated in the Cart entity. This business logic is called from the service, and then the entity is saved, all in a single serive call. However, the Cart and Book entitys must be loaded and apssed to the service before it can be used, so it is not completely encapsulated.
Example 3
public void AddBook(long bookId)
{
ICartRepository cartRepository = new CartRepository();
IBookRepository bookRepository = new BookRepository();
Cart cart = cartRepository.GetByUserName(Thread.CurrentPrincipal.Identity.Name);
Book book = bookRepository.GetById(bookId);
cart.AddBook(book);
using (ITransactionProvider transactionProvider = new TransactionProvider())
{
transactionProvider.BeginTransaction();
cartRepository.Save(cart);
transactionProvider.CommitTransaction();
}
}
In this example, the Book is loaded from the ID passed in, the Cart is loaded from the current security principle identiy name, the business logic is called on the Cart (passing in the Book) and then the Cart is saved. This is all encapsulated into one call.Domain
Example 1
public virtual void AddItem(Book book)
{
if (Books == null)
{
Books = new List<Book>();
}
Books.Add(book);
Total = _books.Sum(b=>b.Price);
}
In the examples where the Cart entity had an 'AddBook' or similar method which took a Book entity as a parameter, the logic was encapsulated in the Domain (this example was from before the discount requirement). However, one problem here is that the entity is accessing its own public 'set' properties.
Example 2
public virtual void AddBook(Book book)
{
_books.Add(book);
float discount = ((from curBook in _books select curBook).Distinct().Count() - 1) * 5f;
_total = (from curBook in _books select curBook.Price).Sum() - discount;
}
In this example (which includes the discount requirement), the entity is accessing its private member variables. We can create a rule that the 'set' properties should not be used at all (I will keep them in for NHibernate, although I believe it is possible for NHibernate to use the private members - I will review this). The private member variables should only be ammended through methods that reflect use cases. This was the entities will adhere to OO principles and will reflect the business more acurately.
Unit Tests
[TestMethod]
public void AddingTwoDifferentBooksGivesDiscount()
{
Cart cart = new Cart();
Book book1 = new Book()
{
Id = 1,
AuthorName = "Robert C. Martin",
Title = "Agile Principles, Patterns and Practices in C#",
Price = 35.00f
};
Book book2 = new Book()
{
Id = 2,
AuthorName = "Martin Fowler",
Title = "Patterns of Enterprise Application Architecture",
Price = 30.00f
};
cart.AddBook(book1);
cart.AddBook(book2);
Assert.AreEqual(60.00f, cart.Total);
}
One way of ensuring that the Domain has been developed correctly is to check that tests for business logic can be written against the Domain which touch only the Domain and nothing else, as in this example (of course, if you are following TDD, the tests should be written first).
Summary
The lessons learnt from this training session can be summarised in this following list of rules and guidances:
- All commands which change the state of the business (equivalent to pressing ‘Submit’ button) should be encapsulated in one service call.
- Never have a ‘Save’ service operation which takes an entity (or DTO of an entity) as a parameter.
- Never call methods of entities in the UI.
- Services usually just load entities, call business logic in domain, and save. *
- Command services should have names that reflect use cases, and call methods on entities with a similar name. *
- Business logic should be contained in the entities.
- You should be able to write tests for the business logic that touch the Domain and nothing else.
- Do not use ‘set’ properties of entities – entity state can only be changed through methods that have names that reflect use cases.
* These are guidances rather than hard rules.
