Search code examples
c#api-design

Is pulling up a method into a generic base class a breaking change

I have a library currently structured as this: 

class Base<T> 
{
}

class Derived : Base<int>
{
    public int Foo();
}

class AnotherDerived : Base<string>
{
}

I would like to add a similar Foo() to AnotherDerived and as the implementation would be identical for both, I would like to pull up Foo() into their shared parent class Base like below.

class Base<T> 
{
    public T Foo();
}

class Derived : Base<int>
{
}

class AnotherDerived : Base<string>
{
}

Doing this changes the return type for Foo() from int to T. Changing the return type of a member is listed as a breaking change. But since Derived is generic in Base<T> with T=int the code can still compile, at least for this example.

int foo = myDerived.Foo();

I don't want to introduce breaking changes to my users, so my question is whether this kind of pull up in any way be considered a breaking change? Moving into the base itself should not be a breaking change

Moving a method onto a class higher in the hierarchy tree of the type from which it was removed

An alternative implementation that enables code sharing and should not be a breaking change, but which I would like to avoid if the pull up is non-breaking, could be:

class Base<T> 
{
    protected T Foo();
}

class Derived : Base<int>
{
    public new int Foo() => base.Foo();
}

class AnotherDerived : Base<string>
{
    public new string Foo() => base.Foo();
}
Solution

  • I don't want to introduce breaking changes to my users

    That's smart.

    my question is whether this pull up in any way be considered a breaking change?

    You asked if it could in any way be a breaking change, and the answer is yes, there are some ways it could be a breaking change. However, the ways that it could be a breaking change are obscure and unlikely to be a factor in real-world code.

    Let's start by clarifying what we mean by a breaking change. For our purposes let's say that there are two software packages, Alpha 1 and Bravo 1, where Alpha is a dependency of Bravo. The question is: suppose we have Alpha 2 and we recompile Bravo without changes, but against Alpha 2. If Bravo no longer compiles, or worse, compiles but changes its behaviour, the change to Alpha is said to be breaking Bravo.

    The question then is, can we come up with a Bravo that consumes your two different class libraries, but does not compile in the latter version? I'll assume that all your classes and members are public. Also, we don't need the base class to be generic, so let's simplify that:

    // Alpha 1
public class Base { }
public class Derived : Base { public int Foo() => 0; }

// Alpha 2
public class Base { public int Foo() => 0;}
public class Derived : Base { }

    Here is a Bravo that compiles with v1 but not v2:

    class Bravo
{
  static void M(Action<Base, Derived> a) {}
  static void M(Action<Derived, Base> a) {}
  static void Main()
  {
    M((x, y) => { x.Foo(); });
  }
}

    What happens in v1? We have two choices: either x is Base or x is Derived. The former is impossible because Base has no Foo. Therefore x is unambiguously Derived and the program compiles.

    But with v2, we have two choices: either x is Base or x is Derived and both have Foo, so we cannot unambiguously say which M is meant. Therefore we fall back to a tiebreaker round, but we have no reason to prefer Action<Base, Derived> to Action<Derived, Base> or vice versa. Bravo fails to compile with an ambiguity error.

    EXERCISE: Construct a class Bravo that compiles with both Alpha 1 and Alpha 2, but where overload resolution chooses a different method for each. That's the more subtle kind of breaking change. (Though we hope that changing which method is called does not change the meaning of the program very much; it would be bizarre if there were two methods with the same name on the same class that did very different things!)

    When we added lambdas to C# 3 I had many long meetings in Mads' office discussing this scenario and we decided that it was worth adding these rare-in-practice breaking change scenarios to the language in exchange for the power that came with type inference on lambdas.

    UPDATE: There are other ways in which moving a method to a base class can cause a change in behaviour that does not involve bizarre lambda binding. For example, consider:

    // version 1
class B 
{
  public void M(int x, int y) { Console.WriteLine(1); }
}
class D : B 
{
  public void M(int x, short y) { Console.WriteLine(2); }
}
...
new D().M(1, 1); // 2

    This is a little surprising, but remember that what we have here is a call with three arguments: this is new D(), x and y are both 1. Our choices of overloads are a method that takes D for this, int for x, and short for y, and a method that takes B for this, int for x and int for y.

    In C#, a literal int is automatically convertible to short if it is in the range of a short. So the question then is: which is more important? That we closely match the receiver to this, or that we closely match the compile-time type of the literal to the type of y? C# decides that the receiver is always more important than any argument, and so chooses the more-derived method.

    If we then move the method into the base class:

    // version 2
class B 
{
  public void M(int x, int y) { Console.WriteLine(1); }
  public void M(int x, short y) { Console.WriteLine(2); }
}
class D : B  { }
...
new D().M(1, 1); // 1

    Now the compiler has no difference in the receiver type of the two methods, so that is no longer a tiebreaker. The only tiebreaker that is now available is "should we think of 1 as more like int or short?" and the answer is int, so this prints 1.

    ANOTHER UPDATE:

    The original poster has clarified that the question was intended to specifically solicit opinions on the fact that the method is becoming generic in the updated version; I did not understand that the emphasis was on the genericity, and instead concentrated on the fact that the method was moving to a base class irrespective of whether it was generic or not.

    There are also some potential issues with making a non-generic method into a generic one, or, as in this case, changing a method in a way that preserves its signature but adds a generic substitution step to do so. But as I noted in the earlier part of this answer, those issues should be exceedingly rare in real-world code.

    C# has a last-chance tiebreaker rule that says that if during overload resolution you have two methods that are exactly identical in their parameter types, but that one of those methods got its signature by generic type substitution and the other did not, then the "natural" method wins. That tiebreaking rule can in theory lead to a breaking change with the proposed refactoring, but this situation is extremely rare in realistic code. I tried, but could not easily produce a scenario in which your refactoring would run into this problem. I'll keep playing around with it though, and if I come up with anything plausible I'll add another update.