views:

105

answers:

4

Original Question

Consider the following scenario:


public abstract class Foo
{
    public string Name { get; set; }

    public Foo()
    {
        this.Name = string.Empty;
    }
    public Foo(string name)
    {
        this.Name = name;
    }
}
public class Bar : Foo
{
    public int Amount { get; set; }

    public Bar()
        : base()
    {
        this.Amount = 0;
    }
    public Bar(string name)
        : base(name)
    {
        this.Amount = 0;
    }
    public Bar(string name, int amount) 
        : base(name)
    {
        this.Amount = amount;
    }
}

Is there any more elegant means of chaining the construction so that there is no duplication of code between them? In this example, I end up having to replicate the code to set the value of the Bar.Amount property to the value of the amount parameter in the second constructor. As the number of variables in the class increases, the permutations for construction could get quite complex. It just sort of smells funny.

I did sift through the first couple of pages of my search on this issue, but I wasn't getting specific results; sorry if this is old hat.

Thanks in advance.

UPDATE

So then I was thinking about it backwards, and the following should be my approach:


public abstract class Foo
{
    public string Name { get; set; }
    public string Description { get; set; }

    public Foo()
        : this(string.Empty, string.Empty) { }

    public Foo(string name)
        : this(name, string.Empty) { }

    public Foo(string name, string description)
    {
        this.Name = name;
        this.Description = description;
    }
}
public class Bar : Foo
{
    public int Amount { get; set; }
    public bool IsAwesome { get; set; }
    public string Comment { get; set; }

    public Bar()
        : this(string.Empty, string.Empty, 0, false, string.Empty) { }

    public Bar(string name)
        : this(name, string.Empty, 0, false, string.Empty) { }

    public Bar(string name, int amount)
        : this(name, string.Empty, amount, false, string.Empty) { }

    public Bar(string name, string description, int amount, bool isAwesome, string comment)
        : base(name, description)
    {
        this.Amount = amount;
        this.IsAwesome = isAwesome;
        this.Comment = comment;
    }
}

Thanks so much for the response.

+8  A: 

Yes, you can call one constructor from another in C# by using the this keyword. This is often used to simulate default parameters. Example:

public class Bar : Foo
{
    public int Amount { get; set; }

    public Bar() : this(String.Empty) {}
    public Bar(string name): this(name, 0) {}
    public Bar(string name, int amount) : base(name)
    {
        this.Amount = amount;
    }
}
Mark Byers
This assigns `null` to `Name` in the case that the parameterless constructor is called whereas, based on the `base` class, it appears that `String.Empty` should be assigned to `Name`.
Jason
OK, I changed it to match what he wants, but actually I think it is bad practice to use string.Empty when you actually mean null. In C# You should use null to represent that you don't know the value of a string.
Mark Byers
@Mark Byers: I completely agree with you, but that's a separate issue.
Jason
This is of course a very simple example. But my reasoning behind that choice is usually a defensive approach of not having to worry about NULL cropping up in something I expect to have a a type value.
Superstringcheese
@Superstringcheese: Yikes, that's a smell!
Jason
@Jason: Is it? It seems reasonable to me to be able to perform a string operation on Name and know that I'm always going to get a valid result, without having to constantly check for null values. I declare it as a string, and then in the constructor I ensure that any value which makes it into that variable will be of a valid type. It's equivalent to all that null checking I'd have to do later, isn't it?
Superstringcheese
@Superstringcheese: what if later you variable will be set to null? You'll get NullReferenceException because you don't check for null.
Roman Boiko
Of course, because I don't perform that validation in the property setter in the example. But my point is that I would. So, if I'm verifying the the value cannot be null in the Set for a string field, it saves me the headache of having to do it in every other case. That's my reasoning, anyway. Is there anything fundamentally wrong with that approach?
Superstringcheese
What if somebody calls a constructor passing null inside? `Bar b = new Bar(null);`
Roman Boiko
And most importantly - what is the purpose of having invalid object (with empty name) that behaves **like** valid? So that if you have a problem, you won't notice that.
Roman Boiko
I guess that depends on what you mean by valid. I'd suspect that in most cases, a String.Empty value is a more valid value for a string variable than is a NULL. As I understand it, NULL equals nothing, which is not a string. String.Empty equals a string with no characters occupying it. An empty string is still a valid string - a null is not. I don't know how valuable it is to pick apart ultra-simplistic code intended to illustrate an entirely different point, but I think it's possible to do validation at the Constructor/Set level and be done with it.
Superstringcheese
The point is, even if it takes some effort to validate a fields value at the Constructor/Set level, it can be done. And if it's tough to do right there, then it will be even more prone to risk if you have to replicate that effort in other areas of the code. If I don't ever want a string to be NULL, why not just put the validation logic at the first and final entry points to the variable?
Superstringcheese
So somebody will pass null and will have String.Empty. Of course, it is not a problem, but... Anyway all those comments were only warnings. Any engineering decision has its pros and cons.
Roman Boiko
A: 

The assumption with constructor chaining is that there is one constructor that does the real work, and other constructor chain to that one. Usually it's to set up optional parameters; the constructor that "does it all" takes the full parameter list. Other constructors have a shorter parameter list, and pass defaults when chaining.

public class Foo : Bar
{
    private bool whyFoo;

    public Foo() : this(true)
    {
    }
    public Foo(bool why) : base(why, false)
    {
        whyFoo = why;
    }
}
Cylon Cat
Would the person who voted this down please explain why? It's very rude to vote down and just walk away from it.
Cylon Cat
@Cylon Cat: probably somebody didn't like that you passed why to `Bar` instead of `base`, or assigned its value to `whyFoo` (why?), or provided an answer that doesn't seem to be useful. (that somebody wasn't me..:)
Roman Boiko
...anyway, it would be great to build your code before posting it.
Roman Boiko
Everybody makes mistakes, I do them often. But I suppose it's not a big deal to have one down-vote.
Roman Boiko
I didn't downvote it either, but your inheritance is inverted from my example, and your variables are different, which makes it harder to think about in relation to my question. Additionally, you don't define Bar, so its fields are arbitrary in your example - for instance, I don't know why you're passing two boolean values to the Bar constructor.
Superstringcheese
Apologies for the non-compiling code; I was in a hurry, which is usually bad for correctness in anything. Yes, ": Bar(..." should be ": base(...". Also, there was no code in the original post when I responded. One a separate note, @Roman and others, no a down-vote isn't a big deal, but down-voting and not commenting is pretty much the equivalent of flipping someone off. It's rude, and you certainly wouldn't want to treat a co-worker like that. Communications and interpersonal skills are important in software development, contrary to the way some people treat the job.
Cylon Cat
@Cylon Cat: agree :) Actually, I've been downvoted in a similar way for another answer. I'd suggest you to edit your answer to make it compilable at least.
Roman Boiko
@Roman, done! It should at least compile now.
Cylon Cat
+3  A: 
public class Bar : Foo {
  public int Amount { get; set; } 

  public Bar() : this(null, 0) { }

  public Bar(string name) : this(name, 0) { }

  public Bar(string name, int amount) : base(name){
    this.Amount = amount;    
  }
}
Ray
+1  A: 
public class Bar : Foo {
    public int Amount { get; set; }

    public Bar() : this(0) { }
    public Bar(int amount) : this(String.Empty, amount) { }
    public Bar(string name) : this(name, 0) { }
    public Bar(string name, int amount) : base(name) {
        this.Amount = amount;
    }
}

or

public class Bar : Foo {
    public int Amount { get; set; }

    public Bar() : this(String.Empty, 0) { }
    public Bar(string name) : this(name, 0) { }
    public Bar(string name, int amount) : base(name) {
        this.Amount = amount;
    }
}
Jason