2008-10-30

Usability session with me from ANUG

Its in Danish, sorry.

I volunteered to participate in a usability study of hertz.com.
I was to rent a car in L.A. for a week.

The session was not rehearsed in advance, and there is an audience of about 50 people.

2008-10-01

Data Structures vs Encapsulation

Having read Clean Code I really wanted to blog about data structures (DTO) and encapsulated objects. But I have been busy and someone else beat me to the punch. Uncle Bob also has an article where he outlines his view.

Data Structure Objects

In C# a Data Structure is represented by a class or struct, that has has no logic and exposes all its state through properties(public fields or get/set methods). Its primarily used near the system boundaries, like UI, Service end points, and database access. Anyone can use the exposed data and operate on it, so adding new behavior is easy, but adding new or changing existing data is troublesome.

Encapsulated Objects

Are represented by C# classes/interfaces with methods that operate on data. The data is hidden(encapsulated). Internally the object can have Data Structure Objects as state, and operate on that state. Methods define the operation that can either be a command (changes the state, and returns nothing) or a query (does not change state, and returns something). A Validator could have a Validate command method that validates the internal state, and a ValidationNotifications query that returns the result of the last validation.

Changing the state is easy because it is only know internal in the class, but adding new or changing existing behavior is troublesome, because it changes the contract of the class.

Hybrid Objects

Hybrids often appear because of database and UI concerns. A class exposes it state so that the UI can data bind and the DAL can save the entity. But the same class should also be encapsulated so that the logic only lives with in the object.

So now the class has two distinct responsibilities:

  1. Exposing state for the UI and DAL to use.
  2. Exposing logic for validation etc.

And guess what, if it is combined in a Hybrid Class it has the drawbacks of both, and non of the benefits. So Hybrids should really be avoided, and interesting happen when you don't create hybrids.

Should all my classes be Encapsulated Objects?

NO!

Even though all the cool kids show off their OO skilz, many problems are best solved with a simple Data Structure Class. If your application starts out as a 1 to 1 mapping from the UI to the database tables, there is little need for encapsulated objects. However as the application gets more logic, be sure to place this logic in encapsulated objects. Avoiding the Hybrid trap, this should help make the application more maintainable.

My advice

Keep your data(from a database or the user etc.) in Data Structures and your logic(anything like if/for/while etc) in Encapsulated Classes. The encapsulated Objects are free to use Data Structures directly or contain them as state. Just like adding string, int, bool and any other simple type feels natural, adding a custom made Data Structure as state should also be natural.

2008-09-09

What Goes In Must Come Out

IO, logic and data

In essence all computer systems are about I/O, data is send against the system and data is sent back.
In the simplest form it looks like this, note that there is no logic involved. Raw data is just passed as input, to change the data and then the changed data is sent back out.
SimpleIO 
I would guess that most systems in the world takes this approach, think about the countless spreadsheets laying around with absolutely no logic.

When logic is introduced I usually see the following layer of logic introduced.
LogicIO 
The logic now has 2 distinct responsibilities.

  1. All input is validated against the business rules, before changes to the data.
  2. Exposure and transformation of the data to a specific client.

These 2 responsibilities are very different. When the data store is optimized for one, the other suffers. If a relational database is used as data store, the data schema can be normalize to make input commands more effective, and help keep our data consistent. But a normalized data store is not good for the output to the client, because it often need data from multiple tables. Either a lot of joins are used, or when using Active Record, DDD entities or the like, whole table rows are loaded into ram, even if only a few fields are needed from each table.
De-normalize the data schema on the other hand, makes input commands tricky because multiple versions of the truth can be scattered all over the database. This quickly ends up with inconsistent data.

The logic also suffers from a asymmetry in how code is designed. Business rules are usually object centric, while client transformation is data centric. This creates an object <-> data structure asymmetry, where the code is neither good object or good data structures. Objects want to encapsulate data and operate on it, while data structures want to expose data and want others to operate on it. Both are needed and have valid reasons to be in a system, objects are good at adding data without breaking their contract, while data structures are good at adding new methods without breaking their contract. But hybrids break their contract, when both data and methods are added, so they are good at neither.

Introducing multiple models

A possible solution could look like this. 

IOSeperatedData 
The logic is split up, business rules are applied before the input is saved to the business data model, and an ETL is used to transform the business data to a specific data model. There could be multiple output specific models (for views, reports and service results) and multiple clients(Web app, Service etc.) using them.

So en essence there is a model with logic optimized for changes, and a model with transformations optimized for reads. The general performance of the system should be better, at the cost of using more disk space.

If you think about it, data is transformed to other models already. BI transforms the data into aggregate structures and Full Text Search transforms data into indexes. The new part is to transform relational data optimized for change, into relational data optimized for reads. Also this has radical influence on the code design, because the business rules no longer get polluted with data transformation and data structure behavior.

2008-09-06

How to create fully encapsulated and simple Domain Models

Background

Udi wrote about this topic, but I never really liked the solution. To me it seemed complex and over architected, even the solutions from the commenter's where surprisingly complex (No disrespect for Udi or any of the commenter's intended).

Business Context

The core domain revolves around renting video games. I am working on a new feature to allow customers to trade in old video games. Customers can trade in multiple games at a time so we have a TradeInCart entity that works similar to most shopping carts that everybody is familiar with. However there are several rules that limit the items that can be placed into the TradeInCart. The core rules are:

1. Only 3 games of the same title can be added to the cart.
2. The total number of items in the cart cannot exceed 10.
3. No games can be added to the cart that the customer had previously reported lost with regards to their rental membership.
    a. If an attempt is made to add a previously reported lost game, then we need to log a BadQueueStatusAddAttempt to the persistence store.

A solution

The service layer service looks like this.

   1: using System.Transactions;
   2:  
   3: namespace VideoGameRenting
   4: {
   5:     public class TradeInService
   6:     {
   7:         private readonly Repository _repository;
   8:  
   9:         private GameDTO _gameDTO;
  10:         private TradeInCartDTO _tradeInCartDTO;
  11:         private LineItemDTO[] _lineItemDTOS;
  12:         private GameReportedLostDTO _gameReportedLostDTO;
  13:         private TradeInCart _tradeInCart;
  14:  
  15:         public TradeInService(Repository repository)
  16:         {
  17:             _repository = repository;
  18:         }
  19:  
  20:         //This is an outer boundry of the system.
  21:         //The operation below is a complete operation aganst the system.
  22:         public void AddGameToCart(int gameId, int tradeInCartId)
  23:         {
  24:             using(TransactionScope scope = new TransactionScope(TransactionScopeOption.RequiresNew))
  25:             {
  26:                 LoadAllDataNeeded(gameId, tradeInCartId);
  27:  
  28:                 ApplyBusinessRulesToData();
  29:  
  30:                 SaveDataIfNeeded();
  31:    
  32:                 scope.Complete();
  33:             }
  34:  
  35:             //If it is a GUI application, "redirect" to a method that renders a view with data
  36:         }
  37:  
  38:         private void LoadAllDataNeeded(int gameId, int tradeInCartId)
  39:         {
  40:             _gameDTO = _repository.Find<GameDTO>(gameId);
  41:             _tradeInCartDTO = _repository.Find<TradeInCartDTO>(tradeInCartId);
  42:             _lineItemDTOS = _repository.Query<LineItemDTO>(x => x.TradeInCartId == _tradeInCartDTO.Id);
  43:             _gameReportedLostDTO = _repository.FindBy<GameReportedLostDTO>(
  44:                 x => x.Customer == _tradeInCartDTO.CustomerId && x.GameId == _gameDTO.Id);
  45:         }
  46:  
  47:         private void ApplyBusinessRulesToData()
  48:         {
  49:             _tradeInCart = new TradeInCart(_tradeInCartDTO, _lineItemDTOS);
  50:             _tradeInCart.AttemptToAddGameAndLogAnyAbuse(_gameDTO, _gameReportedLostDTO);
  51:         }
  52:  
  53:         private void SaveDataIfNeeded()
  54:         {
  55:             _repository.Save(_tradeInCartDTO);
  56:             foreach (LineItemDTO line in _tradeInCart.GetItems())
  57:             {
  58:                 _repository.Save(line);
  59:             }
  60:             foreach (AbuseLogEntryDTO abuseLogEntry in _tradeInCart.GetAbuseLogEntries())
  61:             {
  62:                 _repository.Save(abuseLogEntry);
  63:             }
  64:         }
  65:     }
  66: }

Basically I treat data as pure data structures(DTO) and I have objects that operate on the data.

The TradeInCart DDD Entity looks like this, it is the agregate root for TradeInCartDTO, LinetemDTO and AbuseLogEntryDTO.

   1: using System;
   2: using System.Collections.Generic;
   3:  
   4: namespace VideoGameRenting
   5: {
   6:     public class TradeInCart
   7:     {
   8:         private readonly TradeInCartDTO _tradeInCartDTO;
   9:         private readonly List<LineItemDTO> _items;
  10:         private readonly List<AbuseLogEntryDTO> _abuseLogEntries;
  11:  
  12:         public TradeInCart(TradeInCartDTO tradeInCartDTO, IEnumerable<LineItemDTO> items)
  13:         {
  14:             _tradeInCartDTO = tradeInCartDTO;
  15:             _items = new List<LineItemDTO>(items);
  16:             _abuseLogEntries = new List<AbuseLogEntryDTO>();
  17:         }
  18:  
  19:         public IEnumerable<LineItemDTO> GetItems()
  20:         {
  21:             return _items.AsReadOnly();
  22:         }
  23:  
  24:         public IEnumerable<AbuseLogEntryDTO> GetAbuseLogEntries()
  25:         {
  26:             return _abuseLogEntries.AsReadOnly();
  27:         }
  28:  
  29:         public void AttemptToAddGameAndLogAnyAbuse(GameDTO game, GameReportedLostDTO gameReportedLost)
  30:         {
  31:             if (GameHasBeenReportedLost(gameReportedLost))
  32:             {
  33:                 LogAbuse();
  34:                 return;
  35:             }
  36:             if (GameCannotBeAddedToCart(game)) return;
  37:  
  38:             AddGameToNewLineItem(game);
  39:         }
  40:  
  41:         private bool GameHasBeenReportedLost(GameReportedLostDTO gameReportedLost)
  42:         {
  43:             return gameReportedLost != null;
  44:         }
  45:  
  46:         private void LogAbuse()
  47:         {
  48:             AbuseLogEntryDTO abuseLogEntry = new AbuseLogEntryDTO();
  49:             abuseLogEntry.Message = "BadQueueStatusAddAttempt";
  50:             abuseLogEntry.TimeStamp = DateTime.Now;
  51:             _abuseLogEntries.Add(abuseLogEntry);
  52:         }
  53:  
  54:         private bool GameCannotBeAddedToCart(GameDTO game)
  55:         {
  56:             return _items.Count > 10 || CountGamesWithSameTitle(game.Id) > 3;
  57:         }
  58:  
  59:         private void AddGameToNewLineItem(GameDTO game)
  60:         {
  61:             LineItemDTO lineItemDTO = new LineItemDTO();
  62:             lineItemDTO.GameId = game.Id;
  63:             lineItemDTO.TradeInCartId = _tradeInCartDTO.Id;
  64:             _items.Add(lineItemDTO);
  65:         }
  66:  
  67:         private int CountGamesWithSameTitle(int gameId)
  68:         {
  69:             int gameCount = 0;
  70:             foreach (LineItemDTO line in _items)
  71:             {
  72:                 if(line.GameId == gameId)
  73:                 {
  74:                     gameCount++;
  75:                 }
  76:             }
  77:             return gameCount;
  78:         }
  79:  
  80:  
  81:     }
  82: }

The full source code is in my trunk.

Conclusion

I think my solution is simpler and easier to understand. I think it communicates very well.

I am actually in doubt... Do I need a AddToCartAttempt class, with GameDTO and GameReportedLostDTO as fields and the appropriate logic? Then I would not need to pass around parameters in TradeInCart's GameHasBeenReportedLost, GameCannotBeAddedToCart and AddGameToNewLineItem

2008-08-21

Error creating window handle.

I have been chasing a bug in a VB.NET Windows Forms application today. The error log was filled with Win32Exception Error creating window handle. Digging in the usage log I loacted the following code.

   1: Public Sub DataBindRadioButton(ByVal radioButton As Windows.Forms.RadioButton, ByVal radioButtonTrueValue As Object, ByVal radioButtonFalseValue As Object, ByRef dataSource As Object, ByVal dataMember As String)
   2:      Dim propertyHolder As New propertyHolder(radioButton, radioButtonTrueValue, radioButtonFalseValue, dataSource, dataMember)
   3:      propertyHolder.Name = radioButton.Name & "propertyHolder"
   4:  
   5:      For Each control As control In radioButton.FindForm.Controls
   6:          If control.Name = propertyHolder.Name Then
   7:              radioButton.FindForm.Controls.Remove(control)
   8:              Exit For
   9:          End If
  10:      Next
  11:  
  12:      radioButton.FindForm.Controls.Add(propertyHolder)
  13:  End Sub

propertyHolder is a Control.

So whats the memory leak here?

radioButton.FindForm.Controls.Remove(control), since we remove the control from the container(Form). We now inherit the responsibility to clean up.

So a non leaking version would be.

   1: Public Sub DataBindRadioButton(ByVal radioButton As Windows.Forms.RadioButton, ByVal radioButtonTrueValue As Object, ByVal radioButtonFalseValue As Object, ByRef dataSource As Object, ByVal dataMember As String)
   2:       Dim propertyHolder As New propertyHolder(radioButton, radioButtonTrueValue, radioButtonFalseValue, dataSource, dataMember)
   3:       propertyHolder.Name = radioButton.Name & "propertyHolder"
   4:  
   5:       For Each control As control In radioButton.FindForm.Controls
   6:           If control.Name = propertyHolder.Name Then
   7:               radioButton.FindForm.Controls.Remove(control)
   8:       control.Dispose()
   9:               Exit For
  10:           End If
  11:       Next
  12:  
  13:       radioButton.FindForm.Controls.Add(propertyHolder)
  14:   End Sub

So remember to dispose a control when you dynamically remove it from its container.

The application also had a while loop that in some conditions never exited, and called the above method.