If all you want to do is create class X with certain arguments, subclassing is an odd way of expressing that intent, because you aren’t using any of the features that classes and inheritance give you. It’s not really an anti-pattern, it’s just strange and a bit pointless (unless you have some other reasons for it). A more natural way of expressing this intent would be a Factory Method, which in this case is a fancy name for your “helper method.”

Regarding the general intuition, both “more specific” and “a limited range” are potentially harmful ways of thinking about subclasses, because they both imply that making Square a subclass of Rectangle is a good idea. Without relying on something formal like LSP, I would say a better intuition is that a subclass either provides an implementation of the base interface, or extends the interface to add some new functionality.

Which of these intuitions is correct?

Your coworker is correct (assuming standard type systems).

Think about it, classes represent possible legal values. If class A has one byte field F, you might be inclined to think that A has 256 legal values, but that intuition is incorrect. A is restricting “every permutation of values ever” to “must have field F be 0-255″.

If you extend A with B which has another byte field FF, that is adding restrictions. Instead of “value where F is byte”, you have “value where F is byte && FF is also byte”. Since you’re keeping the old rules, everything that worked for the basetype still works for your subtype. But since you’re adding rules, you’re further specializing away from “could be anything”.

Or to think of it another way, assume that A had a bunch of subtypes: B, C, and D. A variable of type A could be any of these subtypes, but a variable of type B is more specific. It (in most type systems) cannot be a C or a D.

Is this an anti-pattern?

Enh? Having specialized objects is useful and not clearly detrimental to code. Achieving that result by using subtyping is perhaps a little questionable, but depends on what tools you have at your disposal. Even with better tools this implementation is simple, straightforward and robust.

I would not consider this an anti-pattern since it is not clearly wrong and bad in any situation.

No, it’s not an anti-pattern. I can think of a number of practical use cases for this:

1. If you want to use compile time checking to make sure collections of objects only conform to one particular subclass. For example, if you have MySQLDao and SqliteDao in your system, but for some reason you want to make sure a collection only contains data from one source, if you use subclasses as you describe you can have the compiler verify this particular correctness. On the other hand, if you store the data source as a field, this would become a run-time check.
2. My current application has a one to one relationship between AlgorithmConfiguration + ProductClass and AlgorithmInstance. In other words, if you configure FooAlgorithm for ProductClass in the properties file, you can only get one FooAlgorithm for that product class. The only way to get two FooAlgorithms for a given ProductClass is to create a subclass FooBarAlgorithm. This is okay, because it makes the properties file easy to use (there is no need for syntax for many different configurations in one section in the properties file), and it’s very rare there will be more than one instance for a given product class.
3. @Telastyn’s answer gives another good example.

Bottom line: There isn’t really any harm in doing this. An Anti-pattern is defined to be:

An anti-pattern (or antipattern) is a common response to a recurring problem that is usually ineffective and risks being highly counterproductive.

There is no risk of being highly counterproductive here, so it’s not an anti-pattern.

If something is a pattern or an antipattern depends significantly on what language and environment you are writing in. For example, for loops are a pattern in assembly, C, and similar languages and an anti-pattern in lisp.

Let me tell you a story of some code that I wrote long ago…

It was in a language called LPC and implemented a framework for casting spells in a game. You had a spell superclass, which had a subclass of combat spells that handled some checks, and then a subclass for single target direct damage spells, which was then subclassed to the individual spells – magic missile, lightning bolt and the like.

The nature of how LPC worked was that you had a master object that was a Singleton. before you go ‘eww Singleton’ – it was and it wasn’t “eww” at all. One didn’t make copies of the spells, but rather invoked the (effectively) static methods in the spells. The code for magic missile looked like:

inherit "spells/direct";

void init() {
  ::init();
  damage = 10;
  cost = 3;
  delay = 1.0;
  caster_message = "You cast a magic missile at ${target}";
  target_message = "You are hit with a magic missile cast by ${caster}";
}

And you see that? Its a constructor only class. It set some values that were in its parent abstract class (no, it shouldn’t use setters – its an immutable) and that was it. It worked right. It was easy to code, easy to extend and easy to understand.

This sort of pattern translates to other situations where you have a subclass that extends an abstract class and sets a few values of its own, but all of the functionality is in the abstract class it inherits. Go glance at StringBuilder in Java for an example of this – not quite constructor only, but I’m pressed to find much logic in the methods themselves (everything is in the AbstractStringBuilder).

This is not a bad thing, and it is certainly not an anti-pattern (and in some languages it may be a pattern itself).

The most common use of such subclasses that I can think of is exception hierarchies, although that’s a degenerate case where we would typically define nothing at all in the class, as long as the language lets us inherit constructors. We use inheritance to express that ReadError is a special case of IOError, and so on, but ReadError need not override any methods of IOError.

We do this because exceptions are caught by checking their type. Thus, we need to encode the specialisation in the type so that someone can catch only ReadError without catching all IOError, should they want to.

Normally, checking the types of things is terribly bad form and we try to avoid it. If we succeed in avoiding it, then there’s no essential need to express specialisations in the type system.

It might still be useful though, for example if the name of the type appears in the logged string form of the object: this is language and possibly framework specific. You could in principle replace the name of the type in any such system with a call to a method of the class, in which case we would override more than just the constructor in SystemDataHelperBuilder, we’d also override loggingname(). So if there’s any such logging in your system you could imagine that although your class literally only overrides the constructor, “morally” it’s also overriding the class name, and so for practical purposes it isn’t a constructor-only subclass. It has desirable different behaviour, it’s just that you get this behaviour for free without writing any method overrides thanks to cunning use of reflection elsewhere.

So, I would say that his code can be a good idea if there’s some code elsewhere that somehow checks for instances of SystemDataHelperBuilder, either explicitly with a branch (like exception catching branches based on type) or perhaps because there’s some generic code that will end up using the name SystemDataHelperBuilder in a useful way (even if it’s just logging).

However, using types for special cases does lead to some confusion, since if ImmutableRectangle(1,1) and ImmutableSquare(1) behave differently in any way then eventually someone is going to wonder why and perhaps wish it didn’t. Unlike exception hierarchies, you check whether a rectangle is a square by checking whether height == width, not with instanceof. In your example, there’s a difference between a SystemDataHelperBuilder and a DataHelperBuilder created with the exact same arguments, and that has potential to cause problems. Therefore, a function to return an object created with the “right” arguments seems to me normally the right thing to do: the subclass results in two different representations of “the same” thing, and it only helps if you’re doing something you normally would try not to do.

Note that I’m not talking in terms of design patterns and anti-patterns, because I don’t believe that it’s possible to provide a taxonomy of all good and bad ideas that a programmer has ever thought of. Thus, not every idea is a pattern or anti-pattern, it only becomes that when someone identifies that it occurs repeatedly in different contexts and names it 😉 I’m not aware of any particular name for this kind of subclassing.

The strictest Object Oriented definition of a subclass relationship is known as «is-a». Using the traditional example of a square and a rectangle, a square «is-a» rectangle.

By this, you should use subclassing for any situation where you feel a «is-a» is a meaningful relationship for your code.

In your specific case of a subclass with nothing but a constructor, you should analyze it from a «is-a» perspective. It is clear that a SystemDataHelperBuilder «is-a» DataHelperBuilder, so the first pass suggests it is a valid use.

The question that you should answer is whether any of the other code leverages this «is-a» relationship. Does any other code attempt to distinguish SystemDataHelperBuilders from the general population of DataHelperBuilders? If so, the relationship is quite reasonable, and you should keep the subclass.

However, if no other code does this, then what you have is a relationshp that could be a «is-a» relationship, or it could be implemented with a factory. In this case, you could go either way, but I would recommend a Factory rather than a «is-a» relationship. When analyzing Object Oriented code written by another individual, the class hierarchy is a major source of information. It provides the «is-a» relationships they will need to make sense of the code. Accordingly, putting this behavior into a subclass promotes the behavior from “something someone will find if they dig into the superclass’s methods” to “something everyone will look through before they even start diving into the code.” Quoting Guido van Rossum, “code is read more often than it is written,” so decreasing readability of your code for upcoming developers is a stiff price to pay. I would elect not to pay it, if I could use a factory instead.

A subtype is never “wrong” as long as you can always replace an instance of its supertype with an instance of the subtype and have everything still work correctly. This checks out as long as the subclass never tries to weaken any of the guarantees the supertype makes. It can make stronger (more specific) guarantees, so in that sense your coworker’s intuition is correct.

Given that, the subclass you made probably isn’t wrong (since I don’t know exactly what they do, I can’t say for sure.) You need to be wary of the fragile base class problem, and you also have to consider whether an interface would benefit you, but that’s true of all uses of inheritance.

I think the key to answering this question is to look at this one, particular usage scenario.

Inheritance is used to enforce database configuration options, like the connection string.

This is an anti pattern because the class is violating the Single Responsibility Principal — It is configuring itself with a specific source of that configuration and not “Doing one thing, and doing it well.” The configuration of a class should not be baked into the class itself. For setting database connection strings, most times I’ve seen this happen at the application level during some sort of event occurring as the application is started up.

I use constructor only subclasses quite regularly to express different concepts.

For example, consider the following class:

public class Outputter
{
    private readonly IOutputMethod outputMethod;
    private readonly IDataMassager dataMassager;

    public Outputter(IOutputMethod outputMethod, IDataMassager dataMassager)
    {
        this.outputMethod = outputMethod;
        this.dataMassager = dataMassager;
    }

    public MassageDataAndOutput(string data)
    {
        this.outputMethod.Output(this.dataMassager.Massage(data));
    }
}

We can then create a subclass:

public class CapitalizeAndPrintOutputter : Outputter
{
    public CapitalizeAndPrintOutputter()
        : base(new PrintOutputMethod(), new Capitalizer())
    {
    }
}

You can then use a CapitalizeAndPrintOutputter anywhere in your code and you know exactly what type of outputter it is. Furthermore, this pattern actually makes it very easy to use a DI container to create this class. If the DI container knows about PrintOutputMethod and Capitalizer it can even automatically create the CapitalizeAndPrintOutputter for you.

Using this pattern is really quite flexible and useful. Yes, you can use factory methods to do the same thing, but then (at least in C#) you lose some of the power of the type system and certainly using DI containers becomes more cumbersome.

Let me first note that as the code is written, the sub-class is not actually more specific that the parent, as the two fields initialized are both settable by client code.

An instance of the subclass can only be distinguished using a downcast.

Now, if you have a design in which the DatabaseEngine and ConnectionString properties were reimplemented by the subclass, which accessed Configuration to do so, then you have a constraint which makes more sense of having the subclass.

Having said that, “anti-pattern” is too strong for my taste; there is nothing really wrong here.

In the example you gave, your partner is right. The two data helper builders are strategies. One is more specific than the other, but they are both interchangeable once initialized.

Basically if a client of the datahelperbuilder was to receive the systemdatahelperbuilder, nothing would break. Another option would have been to have the systemdatahelperbuilder be a static instance of the datahelperbuilder class.