tags:

views:

269

answers:

3

This code:

public class WidgetPlatform
{
    public Widget LeftmostWidget { get; set; }
    public Widget RightmostWidget { get; set; }

    public String GetWidgetNames()
    {
        return LeftmostWidget.Name + " " + RightmostWidget.Name;
    }
}

doesn't contain any repetition worth worrying about, but it isn't particularly robust. Because the Widgets aren't null-checked, we're leaving an opening for a bug. We could do a null check, but that feels like work. Here's what I really want:

public class WidgetPlatform
{
    [Required]
    public Widget LeftmostWidget { get; set; }

    [Required]
    public Widget RightmostWidget { get; set; }

    public String GetWidgetNames()
    {
        return LeftmostWidget.Name + " " + RightmostWidget.Name;
    }
}

Ideally, it would cause a compile error (the best sort of error) if the object was instantiated without setting the Widgets, but that seems like a tall order. Is there a way to make this syntax work that at least throws an error on instantiation? There's a (relatively) obvious way to do it with reflection if all of the null-checked objects inherit from the same type, but without multiple inheritance that will get ugly pretty fast.

+4  A: 

Whats wrong with Constructors ?

public class WidgetPlatform
{
    public Widget LeftmostWidget { get; set; }
    public Widget RightmostWidget { get; set; }

    public WidgetPlatform()
    {
        this.LeftMostWidget = new Widget();
        this.RightMostWidget = new Widget();
    }

    public WidgetPlatform(Widget left, Widget right)
    {
        if(left == null || right == null)
            throw new ArgumentNullException("Eeep!");

        this.LeftMostWidget = left;
        this.RightMostWidget = right;
    }


    public String GetWidgetNames()
    {
        return LeftmostWidget.Name + " " + RightmostWidget.Name;
    }
}
Eoin Campbell
You'd probably also want to add a check for null in the constructor, and throw ArgumentNullException if necessary.
Jon B
First you could make one constructor call the other.. but then.. eventually you might want to consider separating all constructors from everything else to facilitate Dependency Injection (don't construct yourself.. have a factory do it for you..)
FOR
Agree regarding the null check. The whole point of adding the constructor would be to throw an ArgumentNullException.
Chris
Eoin Campbell
A: 

There is a school of thought that it is best to encapsulate your constructors, and it seems that it would be approperiate here. reference:

http://www.amazon.com/Emergent-Design-Evolutionary-Professional-Development/dp/0321509366

You might consider:

public class WidgetPlatform
    {
        /// <summary>
        /// Hide the constructor.
        /// </summary>
        private WidgetPlatform(Widget left, Widget right)
        {
            this.LeftmostWidget = left;
            this.RightmostWidget = right;
        }

        public Widget LeftmostWidget
        {
            get;
            private set;
        }

        public Widget RightmostWidget
        {
            get;
            private set;
        }

        public static WidgetPlatform GetInstance(Widget left, Widget right)
        {
            return new WidgetPlatform(left, right);
        }
    }

In GetInstance you can check for null, and decide what to do about it, or not ... you have lots of options at this point.

Hope that helps ...

Why not just check for null in the constructor? getInstance is a horrid vestigal of java design patterns.
FlySwat
1) The idea of just using constructors was already mentioned. For me to repeat that would be pointless.2) As I said, "There is a school of thought ...". This is simply another approach to consider. The ref'd book explains why.
+3  A: 
public class WidgetPlatform
{
     public Widget LeftWidget
     {
         get;
         private set;
     }

     public Widget RightWidget
     {
         get;
         private set;
     }

     WidgetPlatForm(Widget w1, Widget w2)
     {
         if (w1 == null || w2 == null)
             throw new ArgumentException();

         this.LeftWidget = w1;
         this.RightWidget = w2;          
     }

     // Etc
}
FlySwat