An interesting discussion around a particular Breaking Change [Kit George]

  • Article

So an interesting situation arose today I though I would share with everyone. I'd love your input on this issue as well, if you feel strongly one way or the other.

Curiously, the C# compiler (at least) allows you to write the following code:

public abstract class Test {
int _val;
public Test() {_val = 9;}
public Test(int i) {_val = i;}
public virtual int Value { get { return _val; } }
public override string ToString() { return _val.ToString("N"); }
}

What's so interesting? Well, we have an abstract class with a public .ctor (indeed, two of them). The fact that it's public is a bit of an odd thing, given that no-one can actually instantiate an instance of this class: it's abstract! Now of course, subclasses can reference the public .ctor like so:

class Test2 : Test {
public Test2() : base(12) {}
public Test2(int i) : base(i) {}
}

But the fact the the .ctor is public on Test is misleading. It gives the impression that it can be called from any code, whereas it's only use is the above: from a subclass. Because of this, it would have been far more suitable to write the following code in the base class:

public abstract class Test {
int _val;
protected Test() {_val = 9;}
protected Test(int i) {_val = i;}
public virtual int Value { get { return _val; } }
public override string ToString() { return _val.ToString("N"); }
}

The interesting part comes when we discovered today that some teams have changed APIs which originally had the public .ctor, to instead be protected. Traditionally, reducing the scope of a member in this way is considered a breaking change, but in this particular instance, there should be no consumer who is actually adversely affected: the only APIs that could originally take advantage of the public .ctor were subclasses, and they can still use the protected .ctor.

So the arguments for/against become:

  • Against: There is the off-chance that some code out there has a whacky way of using the public .ctor in some weird way. It's a long-shot, but obviously, whenever you change code, there's the 1-million shot. The benefit of changing from public to protected in this case is solely cleanliness of the API. It doesn't make any functional difference for the key scenario, accessibility from a subclass. On this basis, the change is not worthwhile
  • For: Making a clean API is valuable. It is less confusing to the consumer, and moving forward, ensures people have no doubts to the usage of that API. The chance of someone actually being broken is so small in this situation, that it does not offset the value of having a clean API. Make the change

Where do you sit?