1

Say, I have a class A which has methods m1,m2,m3,m4,m5.... m100.

I am making another simpler class B and I require only m2,m3 and m5 in B.

Should I be ideally using compositions and put a reference of A inside of B and just delegate the functionality ?

or I should just re-write[copy=paste] those 2-3 methods out of class A ?

gnat
  • 21,442
  • 29
  • 112
  • 288
Amogh Talpallikar
  • 1,997
  • 2
  • 26
  • 39
  • 7
    Neither. Class A is most likely far too complicated and that functionality should be separated out into various classes. – Phoshi Oct 03 '13 at 13:41
  • 4
    If you don't have the possibility to refactor A, then I would write an interface B and an adapter for Class A. Actually, what I really would do is take the functionality out of A, put it in B and have A inherit from B. – Pieter B Oct 03 '13 at 13:48
  • I can't touch A. its "legacy" code! – Amogh Talpallikar Oct 03 '13 at 14:00
  • won't B be become unnecessarily complicated object ? – Amogh Talpallikar Oct 03 '13 at 14:05
  • Making a copy of those methods in class B sounds like the least efficient way to solve this since you will have duplicate code. However, can you be sure that the methods in class A will never change? – DFord Oct 03 '13 at 14:05
  • Yes. if we were allowed to touch class A we would have done refactored this behemoth into smaller classes. when I say m1,m2...m100 I dont necessarily mena it actually has 100 methods but a lot of them for sure. It's exaggeration. say somewhere around 20-25! and I only need 2-3. So I was thinking, if its okay to re-use the code in A or to re-write the code in B. – Amogh Talpallikar Oct 03 '13 at 14:12
  • 2
    to work with "untouchable" legacy crap having 100 (!) methods in a single class, consider establishing an [anti-corruption layer](http://programmers.stackexchange.com/q/184464/31260) – gnat Oct 03 '13 at 14:12
  • 2
    actually what I was using was a class had a lot of methods for a cucumber step definitions some limited features in the app. All these features are unrelated but all of these step def are put into one class. and now I dont want to add more stuff to this class but I still need the existing code as it has some scenarios which are a pre-requisite to the scenario I am automating for testing. – Amogh Talpallikar Oct 03 '13 at 14:17

1 Answers1

1

Ideally, don't cut and paste code. Duplicating functionality inevitably leads to errors in one and a slow divergence of hacks (need to make an 'easy' change in one leading to an 'ugly' change in the other). The key to this is to refactor it in such a way that the ugliness in the original class can be hidden away as best as possible, and a new layer of code in front of the ugly leads to 'abandon in place' of parts of the ugly.

The ultimate goal, despite the 'legacy' nature of A is to dry up the code so that B and A share a common base class. Even if you can't get there, that is the ultimate destination.

The first step in this process is to make that abstract class that implements just the subset of common methods to B and A.

Once you have this, you make a pair of implementation classes - B and A'. A' is an anti-corruption layer / wrapper that wraps around A and passes through the method calls as appopriate.

(This is where it gets a little ugly)

Without knowing if the 5 methods (that are part of the abstract class) change the state of A in a way used by the other 20 methods it can be difficult to consider the proper design.

Lets call all the methods a .. z. The methods a, e, i, o, and u are the ones that are to be implemented in class B. If, in A, the method a modified state used by b and e - then it is probably best that a and e are both pass through.

On the other hand, if aeiou do not change any state used by the other methods, they can be completely encapsulated within the abstract class.

Now, if you can make a change to the legacy code - its a rather minor one - make an interface (iA)that defines every method in A that A implements and A' implements. One can then refactor every passing around of A to iA.

Then, you can change the iA foo = new A() to iA foo = new A'(new A())

Now, you've got a class B that extends the abstract class, A' that extends the abstract class and implements iA which provides identical functionality to A. The code for aeiou is as isolated from A as possible so that changes to it in the abstract class make the appropriate changes to both B and A'.

Community
  • 1