How to extract class?

[Immortal Nephi]

I learn how to use refactoring code. It is interesting. I have a
question how to extract class. I focus to use extract method.
The class Main has one data member. The member function DoOperation_A
() and DoOperation_B() have long methods.


For example:

class Main
{
public:
Main() : data( 0 ) {}
~Main() {}

void DoOperation_A()
{
// Do something to maniuplate data
// Very long method
}

void DoOperation_B()
{
// Do something to maniuplate data
// Very long method
}

private:
int data;
};

I extract DoOperation_A() into four small member functions. I do
extract DoOperation_B() as well. I place them in private. Small
member functions have short methods. They are easier to understand.
They have better readability.


For example:

class Main
{
public:
Main() : data( 0 ) {}
~Main() {}

void DoOperation_A()
{
DoSubOperation_A1();
DoSubOperation_A2();
DoSubOperation_A3();
DoSubOperation_A4();
}

void DoOperation_B()
{
DoSubOperation_B1();
DoSubOperation_B2();
DoSubOperation_B3();
DoSubOperation_B4();
}

private:
void DoSubOperation_A1()
{
// Do something to maniuplate data
}

void DoSubOperation_A2()
{
// Do something to maniuplate data
}

void DoSubOperation_A3()
{
// Do something to maniuplate data
}

void DoSubOperation_A4()
{
// Do something to maniuplate data
}

void DoSubOperation_B1()
{
// Do something to maniuplate data
}

void DoSubOperation_B2()
{
// Do something to maniuplate data
}

void DoSubOperation_B3()
{
// Do something to maniuplate data
}

void DoSubOperation_B4()
{
// Do something to maniuplate data
}

int data;
};

You break one big problem into small pieces of problem. You add more
small member functions to class Main. The class Main grows bigger and
bigger.
Too many member functions have different algorithms. You are
confused to choose which member function belongs. They are too
complex to understand.
I decide to group some member functions. How do you group them?
Extract class is the answer. You move some member functions and data
members from class main to another class A and class B.
The class Main *has a* relationship to both class A and class B. I
have one problem. I can’t move data member to another class because
both class A and class B need to access class Main’s data member
directly. I made my decision not to move data member because I allow
both class A and class B manipulate class Main's data member.
I have two options. First option is to place reference argument into
class A class and class B’s member function’s parameter.
Second option is to use friend class and pointer to class Main. The
class Main allows class A and class B’s member functions to access
class Main’s private data member directly.
I choose to use second option instead.


For example:

class Main;

class A
{
public:
A();
~A();

void DoOperation_A();

private:
void SetPointerToMain( Main &_main );

void DoSubOperation_A1();
void DoSubOperation_A2();
void DoSubOperation_A3();
void DoSubOperation_A4();

Main *pMain;
};

class B
{
public:
B();
~B();

void DoOperation_B();

private:
void SetPointerToMain( Main &_main );

void DoSubOperation_B1();
void DoSubOperation_B2();
void DoSubOperation_B3();
void DoSubOperation_B4();

Main *pMain;
};

class Main
{
friend class A;
friend class B;

public:
Main();
~Main();

void Run();

private:
int data;
A a;
B b;
};

A::A() {}
A::~A() {}

void A::SetPointerToMain( Main &_main )
{
pMain = &_main;
}

void A:oOperation_A()
{
DoSubOperation_A1();
DoSubOperation_A2();
DoSubOperation_A3();
DoSubOperation_A4();
}

void A:oSubOperation_A1()
{
// Do something to maniuplate data
pMain->data += 2 * 1;
}

void A:oSubOperation_A2()
{
// Do something to maniuplate data
pMain->data += 2 * 2;
}

void A:oSubOperation_A3()
{
// Do something to maniuplate data
pMain->data += 2 * 3;
}

void A:oSubOperation_A4()
{
// Do something to maniuplate data
pMain->data += 2 * 4;
}

B::B() {}
B::~B() {}

void B::SetPointerToMain( Main &_main )
{
pMain = &_main;
}

void B:oOperation_B()
{
DoSubOperation_B1();
DoSubOperation_B2();
DoSubOperation_B3();
DoSubOperation_B4();
}

void B:oSubOperation_B1()
{
// Do something to maniuplate data
pMain->data += 4 * 1;
}

void B:oSubOperation_B2()
{
// Do something to maniuplate data
pMain->data += 4 * 2;
}

void B:oSubOperation_B3()
{
// Do something to maniuplate data
pMain->data += 4 * 3;
}

void B:oSubOperation_B4()
{
// Do something to maniuplate data
pMain->data += 4 * 4;
}

Main::Main() : data( 0 )
{
a.SetPointerToMain( *this );
b.SetPointerToMain( *this );
}

Main::~Main() {}

void Main::Run()
{
a.DoOperation_A();
b.DoOperation_B();
}

int main()
{
Main m;
m.Run();

printf( "Done." );

return 0;
}

Please let me know if extract method / class is good practice for
readability. Do you have third option?

 

[Richard]

[Please do not mail me a copy of your followup]

Immortal Nephi <[email protected]> spake the secret code

Please let me know if extract method / class is good practice for
readability. Do you have third option?

You've essentially done "Replace Method with Method Object" (see
Fowler, pg. 135:

"You have a long method that uses local variables in such a way
that you cannot apply Extract Method.

=>

Turn the method into its own object so that all the local
variables become fields on that object. You can then decompose
the method into other methods on the same object."

You seem to be saying that when you did this, the extracted class
needed to access a bunch of the data in the class containing the
original method. If the extracted class is the only place where that
data is referenced, then it should probably be moved to the extracted
class from the original class. If the data is accessed in many
places, then use accessors for this data and pass a reference to the
original class to the extracted class so it can access the data using
the accessors.

You've chosen a solution that uses friendship. My advice is to avoid
friendship whenever possible as it introduces tight coupling.

The questions you are asking are questions of judgment. It is
difficult to respon with specifics because your example doesn't
include the actual code, only an outline of the actual code. Which
approach would be "best" to take in refactoring can only be decided
with actual code.

 

[Immortal Nephi]

[Please do not mail me a copy of your followup]

Immortal Nephi <[email protected]> spake the secret code

   Please let me know if extract method / class is good practice for
readability.  Do you have third option?

You've essentially done "Replace Method with Method Object" (see
Fowler, pg. 135:

        "You have a long method that uses local variables in such a way
        that you cannot apply Extract Method.

        =>

        Turn the method into its own object so that all the local
        variables become fields on that object.  You can then decompose
        the method into other methods on the same object."

Ok...can you please tell me the exact book title name and ISBN. Is it
for C++? You mentioned -- Fowler.

You seem to be saying that when you did this, the extracted class
needed to access a bunch of the data in the class containing the
original method.  If the extracted class is the only place where that
data is referenced, then it should probably be moved to the extracted
class from the original class.  If the data is accessed in many
places, then use accessors for this data and pass a reference to the
original class to the extracted class so it can access the data using
the accessors.

You mention like I described my *first option*. Pass class Main's
data member to class A's member function's parameter in the reference.

You've chosen a solution that uses friendship.  My advice is to avoid
friendship whenever possible as it introduces tight coupling.

The questions you are asking are questions of judgment.  It is
difficult to respon with specifics because your example doesn't
include the actual code, only an outline of the actual code.  Which
approach would be "best" to take in refactoring can only be decided
with actual code.

Like my second option. Consider that class A and class B are
private. Client is able to use class Main only, but they can't see
class A or class B. I continue to study refactoring. Thanks for your
advice.

 

[Ian Collins]

[Please do not mail me a copy of your followup]

Immortal Nephi <[email protected]> spake the secret code

Please let me know if extract method / class is good practice for
readability. Do you have third option?

You've essentially done "Replace Method with Method Object" (see
Fowler, pg. 135:

"You have a long method that uses local variables in such a way
that you cannot apply Extract Method.

=>

Turn the method into its own object so that all the local
variables become fields on that object. You can then decompose
the method into other methods on the same object."

Ok...can you please tell me the exact book title name and ISBN. Is it
for C++? You mentioned -- Fowler.

Google for "Fowler refactoring". Martin's book is *the* book on
refactoring.

 

[Richard]

[Please do not mail me a copy of your followup]

Immortal Nephi <[email protected]> spake the secret code

Ok...can you please tell me the exact book title name and ISBN. Is it
for C++? You mentioned -- Fowler.

The exact reference is "Refactoring: Improving the Design of Existing
Code" by Martin Fowler ISBN-10: 0201485672. Most refactoring is done
on "legacy code", i.e. code without unit tests. Refactoring without
unit tests must be done more carefully because you can't easily tell
if your refactoring changes have broken something. A good companion
to Refactoring is "Working Effectively With Legacy Code" by Michael
Feathers. I wrote a review of Feathers's book here:
<http://wp.me/pyVNs-6>

Fowler's book mostly uses Java as the example language, but the ideas
aren't tied to a particular language. Feathers's book uses Java and
C++ in the examples and some of the catalogued techniques he describes
are only available in one language or another. For instance, if the
technique uses reflection, that won't be a technique you can apply to
C++. Similarly, if the technique uses the preprocessor or link-time
tricks, that won't be a technique you can apply to Java.

The refactoring catalog from Fowler's book is online in an abbreviated
form at <http://refactoring.com>. The book contains more details
about each refactoring, including an example worked from start to
finish. You can read about "Replace Method with Method Object" there at
<http://refactoring.com/catalog/replaceMethodWithMethodObject.html>

For refactorings I've written up, I've tried to follow Fowler's
format. I believe all the refactorings I've written up were in the
context of C++:

You mention like I described my *first option*. Pass class Main's
data member to class A's member function's parameter in the reference.

Yes, you described several alternatives, but as I say its hard to
discuss the merits of the alternatives without specific code in front
of us.

Like my second option. Consider that class A and class B are
private. Client is able to use class Main only, but they can't see
class A or class B. I continue to study refactoring. Thanks for your
advice.

In "C++ Coding Standards", in the introduction to the chapter on class
design and inheritance on pg. 55 they say:

"[...] most of the items in this section are motivated primarily
or exclusively by dependency management. For example, inheritance
is the second-strongest relationship you can express in C++,
second only to friend; it should come as no surprise, then, that
it's important to use such a powerful tool judiciously, correctly,
and well."

If that advice holds true for designs involving inheritance, then it
holds true a hundredfold for designs involving friendship.

In most of my refactoring of legacy C++ code, the uses of friendship
are often just because the person was too lazy to write an accessor or
mutator for data members. So they just made class B a friend of class
A and then B can shove its fingers into the pants of class A as much
as it likes. So much for encapsulation.

Friendship is present in C++ for a reason, but its the design
exception not the design norm. Most of the time you *don't* need
friendship and if you find yourself needing it for simple things like
"Replace Method with Method Object", then you should rethink what
you're doing and consider alternatives.

 

[Immortal Nephi]

[Please do not mail me a copy of your followup]

Immortal Nephi <[email protected]> spake the secret code

Ok...can you please tell me the exact book title name and ISBN.  Is it
for C++?  You mentioned -- Fowler.

The exact reference is "Refactoring: Improving the Design of Existing
Code" by Martin Fowler ISBN-10: 0201485672.  Most refactoring is done
on "legacy code", i.e. code without unit tests.  Refactoring without
unit tests must be done more carefully because you can't easily tell
if your refactoring changes have broken something.  A good companion
to Refactoring is "Working Effectively With Legacy Code" by Michael
Feathers.  I wrote a review of Feathers's book here:
<http://wp.me/pyVNs-6>

Fowler's book mostly uses Java as the example language, but the ideas
aren't tied to a particular language.  Feathers's book uses Java and
C++ in the examples and some of the catalogued techniques he describes
are only available in one language or another.  For instance, if the
technique uses reflection, that won't be a technique you can apply to
C++.  Similarly, if the technique uses the preprocessor or link-time
tricks, that won't be a technique you can apply to Java.

The refactoring catalog from Fowler's book is online in an abbreviated
form at <http://refactoring.com>.  The book contains more details
about each refactoring, including an example worked from start to
finish.  You can read about "Replace Method with Method Object" there at
<http://refactoring.com/catalog/replaceMethodWithMethodObject.html>

For refactorings I've written up, I've tried to follow Fowler's
format.  I believe all the refactorings I've written up were in the
context of C++:

You mention like I described my *first option*.  Pass class Main's
data member to class A's member function's parameter in the reference.

Yes, you described several alternatives, but as I say its hard to
discuss the merits of the alternatives without specific code in front
of us.

Like my second option.  Consider that class A and class B are
private.  Client is able to use class Main only, but they can't see
class A or class B.  I continue to study refactoring.  Thanks for your
advice.

In "C++ Coding Standards", in the introduction to the chapter on class
design and inheritance on pg. 55 they say:

        "[...] most of the items in this section are motivated primarily
        or exclusively by dependency management.  For example, inheritance
        is the second-strongest relationship you can express in C++,
        second only to friend; it should come as no surprise, then, that
        it's important to use such a powerful tool judiciously, correctly,
        and well."

If that advice holds true for designs involving inheritance, then it
holds true a hundredfold for designs involving friendship.

In most of my refactoring of legacy C++ code, the uses of friendship
are often just because the person was too lazy to write an accessor or
mutator for data members.  So they just made class B a friend of class
A and then B can shove its fingers into the pants of class A as much
as it likes.  So much for encapsulation.

Friendship is present in C++ for a reason, but its the design
exception not the design norm.  Most of the time you *don't* need
friendship and if you find yourself needing it for simple things like
"Replace Method with Method Object", then you should rethink what
you're doing and consider alternatives.

Thanks for the information about book. I do not understand what you
are referring "Replace Method with Method Object. The webpage
describes class Order and class PriceCalculator.

I think you are saying extract local variable into three variables.
price is one variable in class Order. primaryBasePrice,
secondaryBasePrice, and tertiaryBasePrice are three variables in class
PriceCalculator.

I do not extract local variable. I said earlier I made my decision
not to move class Main's data member to class A and class B. Both
class A and class B need to access class Main's data member directly.
There is no alternatives, but first option may be the answer.

Let me show you example.

class A
{
public:
void DoOperation_A( int &arg )
{
/* Do something to modify class Main's data member. */
}
};

class Main
{
public:
void Run()
{
DoOperation_A( data );
}

private:
int data;
};

Do you see that DoOperation_A()'s parameter has referance argument?
The DoOperation_A()'s body does not contain local variables. The
algorithm modifies class Main's data member through reference in the
parameter.

Do I explain clear? I guess that you pointed friendship should be
avoided unless it is necessary. Please let me know if you are able to
clarify.

 

[Richard]

[Please do not mail me a copy of your followup]

Immortal Nephi <[email protected]> spake the secret code

Thanks for the information about book. I do not understand what you
are referring "Replace Method with Method Object. The webpage
describes class Order and class PriceCalculator.

The description of "Replace Method with Method Object" is:

You have a long method that uses local variables in such a way
that you cannot apply Extract Method

Turn the method into its own object so that all the local
variables become fields on that object. You can then decompose the
method into other methods on the same object.

This is the same situation you described in your post. It *is* the
refactoring you were performing.

I think you are saying extract local variable into three variables.

Nope. I'm saying you have a situation like this:

class A
{
// ...
void SomeLongMethod();
};

void A::SomeLongMethod()
{
// this is a really long method
}

You started by doing a bunch of Extract Method operations on
SomeLongMethod giving you something like this:

class A
{
// ...
void SomeLongMethod();
void Step1();
void Step2();
void Step3();
};

void A::SomeLongMethod()
{
Step1();
Step2();
Step3();
}

void A::Step1()
{
// extracted code
}

void A::Step2()
{
// extracted code
}

void A::Step3()
{
// extracted code
}

You were saying how this results in a bunch of methods on class A that
are ally only relevant to the SomeLongMethod. "Replace Method with
Method Object" recognizes this and says that instead of just doing
extract method within class A, we move the entirety of SomeLongMethod
to class B and do the Extract Method on class B instead:

class A
{
// ...
void SomeLongMethod();
};

class B
{
public:
void Execute();

private:
void Step1();
void Step2();
void Step3();
};

void A::SomeLongMethod()
{
B executor;
executor.Execute();
}

void B::Execute()
{
Step1();
Step2();
Step3();
}

void B::Step1()
{
// extracted code
}

void B::Step2()
{
// extracted code
}

void B::Step3()
{
// extracted code
}

 

[Paul N]

In most of my refactoring of legacy C++ code, the uses of friendship
are often just because the person was too lazy to write an accessor or
mutator for data members.  So they just made class B a friend of class
A and then B can shove its fingers into the pants of class A as much
as it likes.  So much for encapsulation.

I'm not am expert, but I would have thought that it depended on the
circumstances. Making B a friend means that you have to keep B updated
with any changes you make to A. However, if you instead solve the
problem by writing a mutator for a data member of A, then all the
world knows that the variable (or something like it) is there and is
free to muck about with it. Plus, you can't then change A so as to not
use that concept at all, without breaking all the other code - you
have to keep the variable (or a pseudo-variable) in place, restricting
any possible re-design.