views:

316

answers:

5

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;
            }
        }
    }
}
+1  A: 

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.

Jon B
A: 

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).

Mitch Wheat
A: 

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.

tanascius
I actually made a framework that does this (PHP) and it is just easy to work with, e.g. you just create an object and delete it, two lines finsihed, it no object was actually created, then deleting it doesn't do anything and everything is ok anyway, same with showing an object, if the user asks for an object and sees a blank where it should be displayed, then it is clear that it doesn't exist, this all made developing with this framework very quick
Edward Tanguay
+5  A: 

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.

Jon Skeet
yes, handling all the nulls all the way up would be tedious, good point
Edward Tanguay
+1  A: 

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.

Anthony Mastrean
had not thought about this in terms of ActiveRecord, thanks
Edward Tanguay