If .Create() can't instantiate, should it return empty object, null, or throw an exception?

Question

I want to be able to instantiate any object in my application with this kind of code:

SmartForm smartForm = SmartForm.Create("id = 23");
Customer customer = Customer.Create("id = 222");

I'm now debating what Create() should return if that object does not exist.

• if Create() returns an empty object, then this is kind of a "null pattern" and I can still pass that object around my application and call methods on it, which makes programming with this model convenient and easy

• if Create() returns null, then I need to check after every instantiation if the object equals null or not, which makes programming a bit more tedious but more explicit. A problem with this is that if you forget to check the null, your application could work for a long time without you knowing that you're not checking null, then break all of a sudden

• if Create() throws an exception, it is basically the same as returning null but makes programming even more tedious by making you create a try, next, finally block for each instantiation, but you could throw various types of exceptions (which you can't with the null solution) which could bubble up so that you could more explicitly handle the deep errors on your UI, so this I would think is the most robust solution although would produce try/catch code bloat

So it seems to be a lightness/robustness trade off. Does anyone have any experience of making a decision along these lines where you experienced advantages or disadvantages because of that decision?

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace TestFactory234.Models
{
    public class SmartForm : Item
    {
        public string IdCode { get; set; }
        public string Title { get; set; }
        public string Description { get; set; }
        public int LabelWidth { get; set; }

        public SmartForm() { }

        private SmartForm(string loadCode)
        {
            _loadCode = loadCode;
            TryToInstantiateFromDatabase();
        }

        public static SmartForm Create(string loadCode)
        {
            SmartForm smartForm = new SmartForm(loadCode);

            if (!smartForm.IsEmpty())
            {
                return smartForm;
            }
            else
            {
                return null;
            }
        }
    }
}

Answer 1

If it's foreseeable that creation might fail, but a lot of application code is going to expect that it will work, you really should implement two versions of the creation method, one of which should either succeed or throw an exception, and the other of which should indicate expected failures by some means other than throwing an exception, and throwing an exception only for failures the caller probably isn't anticipating (e.g. CpuIsOnFireException). A common pattern for doing this is to have a TryCreate method return a Boolean indicating success, storing the created argument to a by-ref argument. I don't really like that pattern, since it provides no means of indicating anything other than a pass-fail status (i.e. nothing about why it failed). I think it might be better to have a "try" function return either the new thing or null, but have a by-ref argument indicating the reason for failure. Note that this approach would allow better use of implicit typing (e.g. the "var" keyword in C#).

Answer 2

Your sample code calls a "try load from DB" method and the Create convention looks a lot like a Castle ActiveRecord object. Why not separate the Get/Create database operation, allow the framework to do the work and rely on their conventions?

[ActiveRecord("Forms")]
public class SmartForm : Item
{
    [PrimaryKey("Id")]
    public string IdCode { get; set; }
    [Property]
    public string Title { get; set; }
    [Property]
    public string Description { get; set; }
    [Property]
    public int LabelWidth { get; set; }
}

You get/create instances like this

SmartForm entity = ActiveRecordMediator<SmartForm>.Find(1);
SmartForm otherEntity = ActiveRecordMediator<SmartForm>.FindFirst(/* criteria */);

There are a lot of other methods available for finding instances. I think you'll find that ActiveRecord's defaults regarding throwing exceptions, returning null, or in the case of collections, an empty collection, are very consistent and well implemented.

Answer 3

It depends - if it fails because something is definitely wrong, then an exception makes programming easier - you don't write a try/catch around each call, you just let the exception bubble up. Compare that with checking for a null/blank return value and then throwing an exception.

This sounds like the right time to use ArgumentException, IMO.

If you find yourself creating try/catch "bloat", have a look at why you really need to catch the exceptions rather than just letting them bubble up.

Answer 4

When the default behaviour is to create an instance, then the exceptional behavior is to fail with the creation -> Exception

What would you do with an EmptyObject? You say you can still pass it around - makes this really sense? What do the methods do, when you call them?

Maybe you should implement a second TryCreate() method.

I'd implement the Exception-variant, but it depends on the behaviour you need.

Answer 5

If you are creating a form factory, you would be better passing an enumeration rather than a string (unless of course, this represents a plug-in architecture).

Answer 6

If it returns a blank object, then you'll proceed assuming it's valid - or have to do some kind of elaborate test to see if it's blank or not. If it returns null you'll always have to check for null (and maybe that's fine). I would prefer that it threw an exception - assuming that your code is designed so that this isn't supposed to happen. If this is a normal scenario, then null might be a better choice.