1

Here is the problem illustrated using an example of an immutable class. A Book must have at least one of a title and an ISBN.

public class Book
{
    private readonly string _title;
    private readonly int? _isbn;

    public Book(string title)
        : this(title, null, true)
    {
        ... throw exception if title is null
    }

    public Book(int isbn)
        : this(null, isbn, true)
    {
        ... throw exception if isbn < 1
    }

    public Book(string title, int isbn)
        : this(title, isbn, true)
    {
        ... throw exception if title is null
        ... throw exception if isbn < 1
    }

    private Book(string title, int? isbn, bool privateConstructor = true)
    {
        _title = title;
        _isbn = isbn;
        ... more work (beyond the scope of this example)
    }

    ... public properties, etc.
}

See that privateConstructor boolean? I had to throw that in to differentiate between the public Book(string, int) and the private Book(string, int?). While this is fairly harmless since the extraneous parameter is in a private constructor, its use seems smelly to me. Reordering the parameters of the private constructor would work but that would make the constructor initializers appear non-intuitive (in my real work, there's more than just two parameters in play). What is a better way?

1
  • 2
    This is one reason why factory methods or entire factories are better than constructors: you're free to choose the name, so you're not limited to just one method with the same argument types. Nov 6, 2015 at 7:15

4 Answers 4

1

This looks like C# with optional parameters.

Since you're already throwing exceptions in the public constructors, you can dispense with all those constructors except for the private one. Make it public, make title optional, remove the boolean parameter, and have your callers use named parameters.

The other thing you can do is just make the private constructor an ordinary method call instead, and give it a different name. That way, you don't need the bool parameter to disambiguate it.

1
  • I'm avoiding the use of default parameters in public constructors. However, I've marked your second answer as the answer. I didn't think it was possible to set readonly fields outside of the constructor body, but this thread shows how.
    – krallus
    Nov 8, 2015 at 4:08
1

There isn't really a different way.

Constructors always have a pre-determined name (either something like __init__ as in Python, or the class name as in C++/Java/C#), so that can't be varied to differentiate the two constructors. That means that only the argument list can be used to create different overloads.

If you want to delegate some work to another (private) constructor that happens to need a signature compatible with that of a public constructor, then you have essentially 3 options:

  1. Add a dummy parameter to the private constructor, so that it becomes a separate overload
  2. Merge the public and private constructor with the equivalent signatures
  3. Instead of delegating to a private constructor, delegate to a private function, and accept that the member-initializations need to be duplicated.

Neither of these options is really elegant, so you should decide on a case-by-case basis which is the least ugly.

1
  • Thank you for the reply. I felt it was insightful. modiX' answer demonstrated how #2 can work in this instance.
    – krallus
    Nov 8, 2015 at 3:58
1

Remove your private constructor and do the logic in the last public constructor:

public class Book
{
    private readonly string _title;
    private readonly int? _isbn;

    public Book(string title)
        : this(title, 0)
    {
    }
    public Book(int isbn)
        : this(null, isbn)
    {
    }

    public Book(string title, int isbn)
    {
        ... throw exception if title is null AND isbn < 1
        ... throw exception if isbn < 0

        _title = title;
        _isbn = isbn > 1 ? isbn : null;
        ... more work (beyond the scope of this example)
    }
}

I would use the 0 to indicate the ISBN is not given. Alternatively, it might be better to make _isbn not nullable, because the initial value 0 would indicate there is no ISBN. So:

public class Book
{
    private readonly string _title;
    private readonly int _isbn;

    public Book(string title)
        : this(title, 0)
    {
    }
    public Book(int isbn)
        : this(null, isbn)
    {
    }

    public Book(string title, int isbn)
    {
        ... throw exception if title is null AND isbn < 1
        ... throw exception if isbn < 0

        _title = title;
        _isbn = isbn;
        ... more work (beyond the scope of this example)
    }
}

Better for readability.

1
  • Great suggestion. The only drawback I can see is that in the way this is structured, one can't throw a differentiated exception based on whether it should be just the title, just the isbn, or both the title and isbn being required for a given constructor.
    – krallus
    Nov 8, 2015 at 4:01
0

Is there any reason you cannot use static methods (something like createFromTitle and createFromISBN) + private constructructor for what you are trying to achieve? Just use those static methods as contructors.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.