Method signatures in Java

[tsdh]

In the pull-up method section of the almost final case description, you say:

"Note that a signature consists of a name and a parameter list. The return type is not part of the signature as methods with identical names, but different return types are not allowed in Java."

The first sentence is correct but the second is not. The return types of overriding methods must be identical to or subtypes of the return types of the overridden methods, i.e., the return types must be covariant. So this is perfectly legal:

abstract class A {
    abstract Number foo();
}
class B extends A {
    Integer foo() {
        return 17;
    }
}
class C extends A {
    Double foo() {
        return 42.0;
    }
}

So now I'm a bit confused. The PG TMethodSignature captures the return type but in pull-up you say it should not be considered? So the pattern cannot simply assume that the methods have an identical signature but maybe multiple similar signatures which differ only in the return type, and in the latter case, that difference has to be covariant wrt. the inheritance hierarchy?

Well, I'd suggest we let signatures capture also the return type and stick to identical signatures for the refactoring to trigger. And then, please remove that confusing text which explicitly tells that in java, the return type is not part of the signature. You may add a footnote telling that you are aware of that fact but that the case assumes that as a simplification. Else, i.e., without the return type as part of the sig, the following would be pull-up-able which is clearly wrong:

class A{}
class B extends A {String m() {}}
class C extends A {int m() {}}

[SvenPeldszus]

You are right the second sentence has to say "have to be covariant".

I have seen this mistake already, but not committed the change yet. The return type belongs to the TMethodDefinition.

In TTC a solution Ideally takes care about covariant return types but has not, as this cannot be determined in all cases.

edit: By the way the multiplicities of the returnType are of course [0..1] and not just [1] .

[tsdh]

Well, a pull-up is only possible if the return types are identical. If they are not, then the methods are not semantically equivalent. No need to check for covariance or a common supertype of return types (which always exists anyway: Object).

[SvenPeldszus]

As the returnType now belongs to the TMethodDefinition and therefore to the behavior a solution does not have to care about return types.

"As proving that two implementations have identical behavior is undecidable, this decision has to be taken by a developer before initiating the refactoring."

[tsdh]

No? I'd still assert that the to-be-pulled-up method definitions need to have the same return type.

class A {}
class B extends A {
  int canBePulledUp() {}
  String cannotBePulledUp() {}
  Integer canMaybeBePulledUp() {}
}
class C extends A {
  int canBePulledUp() {}
  int[] cannotBePulledUp() {}
  Number canMaybeBePulledUp() {}
}

Here, canBePulledUp() can definitely be pulled up asserting the behavior is equivalent. cannotBePulledUp() can definitely not be pulled up because of completely separate return types. And canMaybeBePulledUp() can be pulled up only in case all callers of B.canMaybeBePulledUp() treat the return type as Number, or in case C.canMaybeBePulledUp() actually returns an Integer. But both cases are hardly decidable because that information is not contained in the PG.

[SvenPeldszus]

PullUp of canMayBePulledUp() is possible if the definition returning Integer is kept. In this case the introduction of additional explicit casts is not necessary. Of course the implementation of Integer casted to Number must have the identical bevaior as a direct Instance of Number. Overridden methods are dangerous at this point.

For not user defined types this is really not decidable on the PG, as we do not know the inheritance relations.

At this point the requirement of equal return types for TTC is really the easier way. For the standard case a conservative behavior in this situation is allowed and correct.

However, a handling of such interesting cases is definitely not false - e.g. below is a pull-up of random() taking the restrictions from above into account possible and all necessary information are available in the PG.

class OneValue {
  double a;
}
class TwoValues extends OneValue {
  double b;
}

class A{}
class B extends A {
  OneValue getRandom() {}
}
class C extends A {
  TwoValues getRandom() {}
}

[tsdh]

Yes, but in my example Number canMayBePulledUp() in class C could also return a Long or a Double, and then pulling up and keeping the Integer-returning version would be false.

But yes, in your example pulling up the version of C would be ok.