Whole Tomato Software Forums
Whole Tomato Software Forums
Main Site | Profile | Register | Active Topics | Members | Search | FAQ
User name:
Password:
Save Password
Forgot your password?

 All Forums
 Visual Assist
 Feature Requests
 Refactoring: Pimpl idiom
 New Topic  Reply to Topic
 Printer Friendly
Author Previous Topic Topic Next Topic  

Drop
New Member

2 Posts

Posted - Jul 01 2013 :  11:21:39 PM  Show Profile  Reply with Quote
First of all I want to thank you guys, your extension is awesome!It is an IDE inside an IDE and for now I'm unlikely can ever code without Visual Assist.

To improve VA even more, I have a little refactoring feature suggestion specific for C++.
Sorry for very long post, but most of it provides helpful (I hope) code

Feature description :
Almost any C++ programmer:
- must prefer composition over inheritance
- apparently using pimpl idiom
- very lazy and dont like to write boilerplate code

In these situations, in class' methods we call member's / pimpl's methods with same name, same return type and same parameters.
Most times we just delegate to member's / pimpl's methods.

Imagine we have class :


// MyClass.h
typedef int Type;
class MyClass
{
	Type m_t;
public:
	MyClass();
	~MyClass();

	void SetVal(const Type& t);
	const Type& GetVal() const;
	void DoStuff();
private:
	void DoStuff1();
	void DoStuff2();
};

// MyClass.cpp
void MyClass::SetVal(const Type& t) { m_t = t; }

const Type& MyClass::GetVal() const { return m_t; }

void MyClass::DoStuff()
{
	DoStuff1();
	DoStuff2();
}

void MyClass::DoStuff1() {}

void MyClass::DoStuff2() {}



Now, we want to hide all underlying implementation details and we use Pimpl Idiom.
After refactoring(total encapsulation) we get:



// MyClass.h
typedef int Type;
class MyClass
{
	// stripped fields
	// added pimpl
	struct Impl;
	Impl* m_pImpl;
public:
	MyClass();
	~MyClass();

	void SetVal(const Type& t);
	const Type& GetVal() const;
	void DoStuff();
	// stripped private methods
};

// MyClass.cpp
struct MyClass::Impl
{
	// MyClass's private fields go here
	Type m_t;

	//  MyClass's public methods implementations
	void SetVal(const Type& t) { m_t = t; }

	const Type& GetVal() const { return m_t; }

	void DoStuff()
	{
		DoStuff1();
		DoStuff2();
	}

	// MyClass's private methods implementations go here
	void DoStuff1() {}

	void DoStuff2() {}
};

MyClass::MyClass() { /* user must allocate m_pImpl here */ }
MyClass::MyClass() { /* user must deallocate m_pImpl here*/ }

// Most important part:
// Boilerplate stuff to connect interface (MyClass) and implementation (MyClass::Impl)
// MyClass's public methods implementations transforms to delegation to Impl
// Note that boilerplates number depends on number of methods and they clearly copy-pasting
void MyClass::DoStuff() { m_pImpl->DoStuff(); } // just call
void MyClass::SetVal(const Type& t) { m_pImpl->SetVal(t); } // pass parameter
const Type& MyClass::GetVal() const  { return m_pImpl->GetVal(); } // pass return


Note, that last 3 functions are very uninteresting.So most programmers will just copy-paste here.
And it is very error - prone: we can accidentally call wrong function and it will be hard to find later.
That is why automatic refactoring here will be cool.

Use cases:

case 1: Pimplify methods
1. User clicks on a method name in MyClass, choses "Refactor", then "Pimplify method"
2. Dialog pops
3. User inputs several options :
- provide name for Impl : $(Impl) (TODO : detect automatically if one already exists ? )
- provide name for m_pImpl : $(m_pImpl) (TODO: detect automatically if one already exists?)
- choose methods to pimplify : $(methodsList)
4. User presses OK
5. VA begin working (see possible implementation):


void PimplifyMethods()
{
	for each $(method) in $(methodsList)
		void PimplifyMethod($(method));
}


case 2: Pimplify whole class
1. User clicks on a class name(MyClass), choses "Refactor", then "Pimplify class".
2. Dialog pops
3. User inputs several options like before
4. User presses OK
5. VA begin working(see possible implementation) :


void PimplifyClass($(Class))
{
	for each $(field) in $(Class).GetFields()
		void PimplifyField($(field))

	for each $(method) in $(Class).GetMethods()
		void PimplifyMethod($(method));
}


case 3: Create method in MyClass from method in MyClass::Impl
1. User clicks on a method name in MyClass::Impl, choses "Refactor", then "Create method in interface class".
2. VA begin working (see possible implementation) :


void MethodFromPimplMethod($(method))
{
	// TODO
	// make method declaration in public section of MyClass
	// MakeBoilerplate($(method));
}


Possible pseudocode implementation:

void AddPimplToClass()
{
	to MyClass private section add :
	struct $(Impl);
	$(Impl) $(m_pImpl);
	// TODO: mark class as pimpled?
}

void PimplifyField($(field))
{
	move MyClass $(field) to MyClass::Impl
}

void MoveMethodToPimpl($(method))
{
	if ($(method) has only declaration in header(no definition))
	{
		move MyClass $(method) declaration from header to MyClass::Impl as public method and add empty implementation
			return;
	}

	if ($(method) has definition in header)
		move $(method) definition from header to MyClass::Impl public section

	if ($(method) has definition in cpp)
		move MyClass::$(method) definition from cpp to MyClass::Impl public section
}
	 
void MakeBoilerplate($(method))
{
	// in cpp create MyClass method's implemetation that calls pimpl method
	if ($(ReturnType) == void))
	{
		Method = $(MethodSignature) { $(m_pImpl)->$(MethodName)($(Parameters)); } // no return
		// Example: Type& MyClass::GetVal() const  { return m_pImpl->GetVal(); }
	}
	else
	{
		Method = $(MethodSignature) { return $(m_pImpl)->$(MethodName)($(Parameters)); } // return
		// Example: void MyClass::SetVal(const Type& t) { m_pImpl->SetVal(t); }
	}
}

void PimplifyMethod($(method))
{
	if ($(method) is private)
	{
		MoveMethodToPimpl($(method));
	}
	else // public and protected
	{
		MoveMethodToPimpl($(method));
		MakeBoilerplate($(method));
	}
}



I hope implement such a feature not very hard, because VA already have similar refactoring options.
Thanks in advance and keep giong!

Edited by - Drop on Jul 01 2013 11:23:01 PM

feline
Whole Tomato Software

United Kingdom
18751 Posts

Posted - Jul 04 2013 :  8:05:00 PM  Show Profile  Reply with Quote
We are considering adding a refactoring command to do this:

case=67156

Thank you for your thoughtful post about this.

zen is the art of being at one with the two'ness
Go to Top of Page

mbussler
Starting Member

Germany
1 Posts

Posted - Dec 08 2016 :  05:13:31 AM  Show Profile  Reply with Quote
Any progress on this one?
I haven't been able to define my own refactoring pattern for this. :(
Go to Top of Page

accord
Whole Tomato Software

United Kingdom
3287 Posts

Posted - Dec 10 2016 :  5:57:39 PM  Show Profile  Reply with Quote
We haven't made any progress so far on this, but it is still on our list of things to consider. I have noted your interest in the case.
Go to Top of Page
  Previous Topic Topic Next Topic  
 New Topic  Reply to Topic
 Printer Friendly
Jump To:
© 2023 Whole Tomato Software, LLC Go To Top Of Page
Snitz Forums 2000