NullReferenceException
is something you have to consider from day one. Writing code on the assumption that references will always be set is leaving the door wide open to bugs, often seemingly intermittent bugs in the future. AFAIK there is no "quick fix" for the problem, but there are re-factoring steps you can take to reduce the impact of Nulls, and help ensure that exceptions around unexpected null values happen at places where the call stack can actually be useful:
#1. Remove all *OrDefault()
methods from Linq expressions where they are not explicitly handling the fact that data may not come back. This means replace FirstOrDefault()
with First()
or better, Single()
where you expect 1 record. You will still get exceptions, but it will be at the point the entity is read making it clear what condition isn't valid vs. later in the code where the entity is used without checking for #null.
#2. Initialize collection properties in the entities. This helps ensure that collections in entities are "ready to go" when creating new entities in case functions receiving entities might get a new row vs. existing one. I.e.
public virtual ICollection<OrderLine> OrderLines { get; set; } = new List<OrderLine>();
#3. Ensure lazy loading is enabled, and mark references and collections as virtual
. This is by no means ideal, but IMHO a lazy load call is better than a NullReferenceException
. If the calls happen to be made outside of the scope of the DbContext an entity was loaded from then you will still get an exception but it will generally be easier to determine what was accessed.
#4. Add Fail-fast validation. When it comes to accepting constructor parameters that are stored, add null check asserts. For methods accepting anything that might be null-able, add asserts. You'll get exceptions, but meaningful ones that will help you identify the source of your bugs. For example:
// Constructor injected references:
oublic OrderService(IOrderRepository repository)
{
// change this:
// _repository = repository;
// ... to this:
_repository = repository ?? throw new ArgumentNullException("repository");
}
// Parameter assertion:
public void AddCustomer(Customer customer)
{
// add these to assert your parameters.
if(customer == null) throw new ArgumentNullException("customer");
// ...
}
These are generally quick re-factors that can be applied quite unobtrusively throughout an existing application.
#5. Aim to eliminate any code that is constructing entities other than to insert a new entity. Common examples I come across are things like:
.Select(x => new Order { /* populating some Order columns */ })
Where Order
is an Entity class but is being used like a view model or some other temporary data container. Any other code that is new
-ing up an entity other than for the purpose of using context.Orders.Add(order);
should be re-factored with prejudice. An entity class should always reflect a complete, or complete-able representation of the data. (Complete-able meaning a tracked entity still within the scope of it's DbContext and able to lazy-load if needed) Any function that gets called accepting an Order entity should expect to receive a complete and valid Order with all references either eager loaded or lazy loadable. All entities passed around should also all be associated to the same DbContext, so code serializing entities to and from web clients should ideally be re-factored to use projections or be very meticulous about ensuring entities are re-associated to the currently scoped DbContext.
Ultimately what you are facing is a form of Technical Debt, and like all debts, the "credit" you got early in terms of saving development time with assumptions accrues interest which will add time later to track down bugs and ultimately "fix" bad assumptions for good. You need to take a Boy Scout's view of the application and aim to leave it cleaner than it was when you started.