Echtzeitsysteme / java-refactoring-ttc

Object-oriented Refactoring of Java Programs using Graph Transformation (TTC'2015)
0 stars 0 forks source link

Why consider constructors? #6

Closed tsdh closed 9 years ago

tsdh commented 9 years ago

The current version of the case description explicitly mentiones that solutions should consider constructors and handle them as methods. What for? Constructors can never ever be pulled up and are unaffected by the other transformation tasks, so I see no reason to have them in the PG.

IMHO, for the case it makes sense to keep the PG as minimal as possible, i.e., have it only cover user-defined (editable) classes and then only the parts of the classes that are affected by the refactorings.

SvenPeldszus commented 9 years ago

You are right constructors can not be pulled up or something like this. However, the call of a constructor results in explicit dependencies and has to be considered in some way.

As it is simpler in this case I use for an example a field which will be pulled up. The same holds for methods but there are more references.

class A{
  void m(){}
}

class B extends A {}

class C extends B {
  C(){
    super.m();
  }
  void m(){}
}

class D {
  A a = new C();
}

edit: removed error from example

At first to clarify what we meant with handling constructors like methods. There should be created a structure in the PG like this for the definition of constructor C() and call at rhs of field a:A:

                     ----type-> TClass(tName="A") 
fSig:TFieldSignature <-signatures--field-> TField(tName=a")
                     <-signature--definitions->fDef:TFieldDefinition
fDef:TFieldDefinition -access-> constrDef:TMethodDefinition
                            <-defines-TClass(tName="C")
constrDef:TMethodDefinition <-definitions--signature-> constrSig:TMethodSignature
                            ---access-> mOfA:TMethodDefinition
constrSig:TMethodSignature <-signatures--method-> TMethod(tName="C")
                           ---returnType->TClass(tName="C")

The constructor in the example to has an access reference to the definition of m() contained in class A. If the constructors are not taken into account the pull-up of m() from C to B would be possible. However this would change the beavior of the program as in the constructor C() a different definition of m() would be called.

Hopefully this ASCII-Art like notation is readable.

tsdh commented 9 years ago

Sorry, I don't get it. You say "If the constructors are not taken into account the pull-up of m() from C to B would be possible. " but C doesn't have a declaration of m(). And C has a constructor B()? I think your code example is not as you intended.

Anyway, of course I see that constructors would need to be considered in the "real world". But we also don't handle statics (or rather we just ignore the modifier and treat them as normal fields/methods), visibility modifiers, or type casts. Such simplifications are perfectly ok for a TTC case. Remember, in the end solution devs have to demonstrate and explain their solution in about 10 minutes, and the proceeding papers will probably be restricted to 5 pages each.

So I'd suggest to exclude constructors and concentrate on normal instance methods. Of course that's your decision but your case is the largest one of this year's three cases anyway.

SvenPeldszus commented 9 years ago

Sorry, the example I meant was.

class A{
  void m(){}
}

class B extends A {}

class C extends B {
  C(){
    super.m();
  }
  void m(){}
}

class D {
  A a = new C();
}

There should not be any additional effort necessary if constructors are just treated as methods, with their according class as return type.

However, a solution which does not handle constructors will not have any major problems. There will be no test cases which tries to pull-up a constructor or something like this.

tsdh commented 9 years ago

Sven Peldszus notifications@github.com writes:

Sorry, the example I meant was.

Ok, I see.

There should not be any additional effort necessary if constructors are just treated as methods, with their according class as return type.

Indeed, that's a complication due to the use of super.

Bye, Tassilo

tsdh commented 9 years ago

Please correct me if I'm wrong but isn't the case in your example undecidable from the information contained in the PG?

class A{
  void m(){}
}
class B extends A {}
class C extends B {
  C(){
    super.m();
  }
  void m(){}
}

The constructor C() (which could also be a normal method) calls super.m(), i.e., it calls B.m(). However, B has no method declaration for m() so effectively the m() method inherited from A is what is called here.

In the PG, method calls and field accesses are only tracked between TMethodDefinitions and TFieldDefinitions. There is no way to represent the situation above, i.e., that C() calls an inherited method. At best, the PG might contain the information that C() calls A.m() and that would allow pulling up m() from C to B.

By the way: I hadn't considered cases as above as pull-up candidates at all because if a subclass overrides a method of a parent class, it does that for a reason and is almost certainly not semantically equivalent to the overridden method. So I had restricted the candidates to methods which aren't overriding a parent method already, and then m() wouldn't be a candidate. (If you think that pulling up methods that already override inherited methods, fine, but at least the case description should mention that explicitly then.)

And additionally: I also hadn't considered this case for the other reason that I had assumed that there must be at least two subclasses declaring a method for pull-up to trigger. Else, in a hierarchy A <- B <- C <- D with D defining a method x(), that method can be pulled up stepwise all the way to the top-level user-defined class A. That makes no sense in my book.

SvenPeldszus commented 9 years ago

As the example is really very constructed and the keyword super has complex drawbacks I will exclude it from the case.

One could try with dummy instances of TMethodDefinitions and the overriding reference but this is not a proper way to handle this.

By the way: I hadn't considered cases as above as pull-up candidates at all because if a subclass overrides a method of a parent class, it does that for a reason and is almost certainly not semantically equivalent to the overridden method. So I had restricted the candidates to methods which aren't overriding a parent method already, and then m() wouldn't be a candidate. (If you think that pulling up methods that already override inherited methods, fine, but at least the case description should mention that explicitly then.)

By the way, this is in my opinion an acceptable assumption.

And additionally: I also hadn't considered this case for the other reason that I had assumed that there must be at least two subclasses declaring a method for pull-up to trigger. Else, in a hierarchy A <- B <- C <- D with D defining a method x(), that method can be pulled up stepwise all the way to the top-level user-defined class A. That makes no sense in my book.

Yes I left the a second child class away as this class was not important for the access reference. However, I forgot to mention it.

tsdh commented 9 years ago

Ok, great.

And another thing. This is a valid class (except that it doesn't conform to Java naming conventions):

class A {
    A() {}
    A A() {return new A();}
}

In this case, the constructor and the method A() would have the same TMethodSignature in the PG. So either we don't consider constructors at all, have a separate PG classes for it, or simply assume that methods are never named like the containing class. In any case, that should be clarified in the case description.

SvenPeldszus commented 9 years ago

Ok, you've got me. I will exclude constructors, too.

tsdh commented 9 years ago

Ok, great. I use JaMoPP for parsing Java, and there constructors and normal methods don't have a common superclass except for Member, so that this decision saves me from having another rule in my jamopp2pg transformation which is almost identical to the method2tmethod rule. :-)