Can I make my constructors less repetitive?

Question

I'm extending a class with 10 different constructors. The new subclass, SpecialImage, is used like this:

SpecialImage specialImage = new SpecialImage(..);

// Leverage the Rotate() method of superclass Image, which
// returns a new rotated image.
// Notice the type is still SpecialImage.
SpecialImage rotated = specialImage.Rotate(...);

SpecialImage implements 5 of the constructors, each of which takes the same additional parameters, Thing stuff and int range:

public class SpecialImage<TDepth> : Image<Gray, TDepth>
    where TDepth : new()
{

    Thing _stuff;
    int _range;

    public SpecialImage(Thing stuff, int range) : base()
    {
        InitParams(stuff, range)
    }

    public SpecialImage(Thing stuff, int range, Size size) : base(size)
    {
        InitParams(stuff, range)
    }

    public SpecialImage(Thing stuff, int range, int width, int height) : base(width, height)
    {
        InitParams(stuff, range)
    }

    public SpecialImage(Thing stuff, int range, string filename) : base(filename)
    {
        InitParams(stuff, range)
    }

    public SpecialImage(Thing stuff, int range, TDepth[,,] data) : base(data)
    {
        InitParams(stuff, range)
    }

    private void InitParams(Thing stuff, int range)
    {
        _stuff = stuff;
        _range = range;
    }
}

This is a tad repetitive, which is only really a problem in the face of maintenance. Every time the construction requirements of SpecialImage change, I have 5 constructors to change.

I sure feel like I should be repeating myself less, but I don't know how to remove this type of redundancy. Is it possible to make this less redundant?

Answer 1

You could use the Builder pattern.

Don't have 10 constructors in Image, have only one which takes all possible parameters. But don't call this constructor from your actual code. Call it from a separate Builder class.

A Builder is a separate class which is initialized with the default values, offers several setters to change these values and a build() method to create a new object with its current values.

Your code to create a new image would then look like this:

ImageBuilder builder = new ImageBuilder();
builder.setStuff(stuff);
builder.setRange(range);
builder.setSize(size); // alternative overload: builder.setSize(width, height);
builder.setFilename(filename);
Image image = builder.build();

Your SpecialImageBuilder would inherit from ImageBuilder. When Image and SpecialImage have the same properties, then likely the only method you need to override is build.

By the way, an elegant trick you can do with builders is to have each setter return this. This is called "fluent interface" and allows you to write code like this:

Image image = new ImageBuilder()
                  .setStuff(stuff)
                  .setRange(range)
                  .setSize(size)
                  .setFilename(filename)
                  .build();

Answer 2

No, Assuming you can't change the base class, or know that one constructor chains to the others. There's no way to choose one of multiple base constructors from another constructor. I thought you might be able to do it with generics, but apparently not.

Even if you use a factory or builder class, where you can call a create method and switch between constructors that it uses within that method, you don't solve your immediate problem. As you still require multiple constructors in your SpecialImage Class

One solution is to wrap your base class rather than inheriting it. This assumes the base class implements an interface, or you can add one on

using System;

namespace LessConstructors
{
    public class Size
    { }
    public class Thing
    { }
    public class Gray
    { }

    public interface IImage
    {
        object MethodX();
    }

    public class Image<TColour, TDepth> : IImage
    {
        public Image() { }

        public Image(Size size) { }

        public Image(int width, int height) { }

        public Image(string filename) { }

        public Image(TDepth[,,] data) { }

        public object MethodX()
        {
            throw new NotImplementedException();
        }
    }

    public interface IConstructor<T1, T2>
    {
        Image<T1, T2> Create();
    }
    public class SizeConstructor<T1, T2> : IConstructor<T1, T2>
    {
        public Size size { get; set; }

        public Image<T1, T2> Create()
        {
            return new Image<T1, T2>(this.size);
        }
    }

    public class HeightAndWidthConstructor<T1, T2> : IConstructor<T1, T2>
    {
        public int Height { get; set; }
        public int Width { get; set; }

        public Image<T1, T2> Create()
        {
            return new Image<T1, T2>(this.Height, this.Width);
        }
    }

    public class SpecialImage<TDepth> : IImage
        where TDepth : new()
    {
        private Image<Gray, TDepth> image;
        Thing _stuff;
        int _range;

        public SpecialImage(Thing stuff, int range, IConstructor<Gray, TDepth> constructor)
        {
            _stuff = stuff;
            _range = range;
            image = constructor.Create();
        }

        public object MethodX()
        {
            return this.image.MethodX();
        }
    }
}

Answer 3

You could use composition as lxrec suggested and then retrieve the delegate when you actually need an Image instance. That might be problematic if you'd like to just use SpecialImage wherever you'd previously be using an Image.

Other than that, can't really see a better solution. The only other suggestion I have is to use named factory methods in lieu of constructors, but that's just for readability.

Answer 4

The main consideration is convenience for the users of your class, which is part of the usability of your API. Too many or too few options could hurt usability. In particular, too many options can confuse users and force them to spend more time choosing. Therefore, you have to consider carefully whether each constructor overload will actually be used. Aggressively remove unused ones before publishing (to internal or external users of the API).

For example, if you provide a constructor that takes a Size, you certainly don't need to provide another constructor that takes int width, int height.

The constructor that takes a string filename is also a bit disingenuous. The colorspace of an image file is decided by the file content. A JPEG file could be gray or YCbCr or YCCK. (YCbCr are customarily auto-converted into RGB upon loading, but there are situations where you need the YCbCr values from the file without the quantization error caused by the conversion.) So, a constructor should not have decided the colorspace without examining the image format first.

I am deeply puzzled by the constructor that doesn't initialize its Image. The rest of your class design seems to imply that its Image must be initialized. Is it your intention to make it a Null Object? If that is not your intention, that constructor shouldn't have existed.

Finally a bit of caution. It looks like you are designing a "single-channel high-dynamic-range" image processing layer on top of an existing library. (Single-channel as in "gray", and high-dynamic-range as in the intention to support 32-bit integer, 32-bit float and 64-bit float.) You might be surprised that some image operations do not support HDR depths at all - in other words, some image operations are limited to TDepth = Byte only. Trying to perform operations on HDR depth might throw an exception at runtime.

I can find a lot of wrongs with the underlying libraries, but I must admit that designing an API for an image library is very difficult. I applaud everyone who made a good effort and shared their work.