Question

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?

Was it helpful?

Solution

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.

OTHER TIPS

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.

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.

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.

Licensed under: CC-BY-SA with attribution
scroll top