Designing a designated constructor

Question

The following pattern is common in my code: A class has several constructors. One is the "designated constructor", the others are for convenience. The code may look like this:

class Foo 
{
  Foo(int bar, string baz) 
  {
    this.Bar = bar;
    this.Baz = baz;
  }

  Foo()
    : this(0, "Empty baz")
  {
  }

  Foo(Foo f)
    : this(f.Bar, f.Baz)
  {
  }

  int Bar {get;set;}
  string Baz {get;set;}
}

In the case of the parameterless constructor calling this(...), this works fine. However, if someone passes null as an argument to the copy constructor Foo(Foo), the result will be an null reference exception because of the expression f.Bar. Because I would like to see an ArgumentNullException instead, I usually stray from the pattern in cases like this and implement the constructor "manually", which results in duplicate code. Is there an elegant way to avoid this, i.e. having one designated constructor and still perform parameter validation for the others?

Edited: This example is bare-bones just to describe the issue. In real-world examples, there will be more complex argument validation logic and initialization code.

You could do in-line null checking before attempting to propagate the property values, but this doesn't help validation and is not elegant. — Adam Houldsworth
– Adam Houldsworth, Commented Jul 17, 2012 at 10:32
Maybe you don't know this, but you can use the following c# syntax: new Foo() { Bar = 1, Baz = "test" };. I've found that it makes those kinds of constructors useless (unless you need required properties of course) — Team-JoKi
– Team-JoKi, Commented Jul 17, 2012 at 10:38
@Team-JoKi That's useful syntax; my example is about constructors with required parameters, as you have guessed (in the example I put "get;set;" to abbreviate the code) — waldrumpus
– waldrumpus, Commented Jul 17, 2012 at 11:31

Rawling · Accepted Answer · 2012-07-17 10:41:02Z

2

You could check this as follows:

class Foo
{
    private static Foo ThrowIfNull(Foo foo)
    {
        if (foo == null) throw new ArgumentNullException("foo");
        return foo;
    }

    ...

    Foo(Foo f) : this (ThrowIfNull(f).Bar, f.Baz)
    {
    }
}

It'll throw your stack trace off a little, but not too much.

answered Jul 17, 2012 at 10:41

Rawling

50.3k7 gold badges94 silver badges131 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Adam Houldsworth Over a year ago

+1 Quite cute, that's a dangerous candidate for object extension methods lol

Tomtom · Accepted Answer · 2012-07-17 10:34:01Z

1

You should always check parameters you get from outside your class-scope.

answered Jul 17, 2012 at 10:34

Tomtom

9,4287 gold badges58 silver badges99 bronze badges

Comments

Matías Fidemraizer · Accepted Answer · 2012-07-17 10:58:29Z

0

Are you sure this code could not be converted to: (?)

  Foo(Foo f)
  {
       if(f != null) 
       {
            Bar = f.Bar;
            Baz = f.Baz;
       }
       else
       {
           throw new ArgumentNullException("f");
       }
  }

Honestly I believe this is the way to go in your case. Why overcomplicating things?

You're looking to set some properties during construction-time, and you can implement this in the constructor itself. Because you want to avoid 1, 2 or 3 lines, you're trying to invent a wheel! ;)

OPTION B ··· UPDATE!!

There's another approach. What about a single constructor covering your use cases but using optional parameters?

Your constructor would be like this:

Foo(int bar = -1, string baz = null, Foo f = null) 
  {
    if(bar >= 0 && !string.IsNullOrEmpty(baz)) 
    {
       Bar = bar;
       Baz = baz;
    }
    else if(f != null) 
    {
       Bar = f.Bar;
       Baz = f.Baz;
    }
    else
    {
        Bar = 0;
        Baz = "Empty baz";
    }
  }

Depending on how you invoke this constructor, it's going to do a thing or other.

edited Jul 17, 2012 at 10:58

answered Jul 17, 2012 at 10:41

Matías Fidemraizer

65.1k19 gold badges132 silver badges216 bronze badges

3 Comments

Alderath Over a year ago

I think that this was just a simple example. The main point was that he wants to have one "main" constructor which does the actual instantiation, and then have all other convenience constructors delegate instantiation to the main constructor. For the simple case of just assigning a variable, copying the assignments from the main constructor (like you did) might be fine, but if the main constructor contains complex logic and there are many convenience constructors, this will result in code copied into multiple locations, which is generally a bad thing.

Matías Fidemraizer Over a year ago

I'm agreed, but we'd see actual use case samples in order to determine the best solution. Sometimes will be fine with my answer, others no.

waldrumpus Over a year ago

@Alderath You're spot on, the real benefit is gained when the constructor contains serious construction logic.

Collectives™ on Stack Overflow

Designing a designated constructor

3 Answers 3

1 Comment

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related