Subject: Re: refactoring AP_*Frame
From: Paul Rohr (paul@abisource.com)
Date: Thu Apr 26 2001 - 06:37:51 CDT
At 10:09 AM 4/26/01 +0200, Hubert Figuiere wrote:
>According to Paul Rohr <paul@abisource.com>:
>>
>> I'm curious. If it's "just" a bunch of cut-and-paste code, why would we
>> need to break the tree to get it refactored? In theory, it should be safe
>> to replace the N duplicate references with the relevant XP call, no?
>>
>Just because in those cut & paste code, the only difference comes
>from platform object instantiation.
Hmm. I guess I'm still missing something here, because this still sounds
like a tree-wide change to me. The most important work in this kind of
refactoring is guaranteeing that each platform can still do what it already
does.
To start with a simple hypothetical example, say that we have six platform
implementations of a particular virtual method that all look more or less
like this:
OS::doFoo(...)
{
/* 50 lines of XP cut-n-paste code */
/* 10 lines of platform-specific code */
/* 100 more lines of XP cut-n-paste code */
}
The idea of refactoring would be to move that identical code out of the OS
implementations into a single XP spot *without* breaking any of them. Thus
a naive end result might be:
XP::doFoo1(...)
{
/* 50 lines of XP cut-n-paste code */
}
XP::doFoo2(...)
{
/* 100 more lines of XP cut-n-paste code */
}
OS::doFoo(...)
{
XP_doFoo1(..)
/* 10 lines of platform-specific code */
XP_doFoo2(..)
}
Note that in this case, the change to each platform would be to delete big
chunks of cut-and-paste code and replace it with an XP call.
However, since we're C++, we could refactor the code differently using
virtual methods, to wind up with something much cleaner, like:
XP::doFoo(...)
{
/* 50 lines of XP cut-n-paste code */
/* call virtual method implemented in platform code */
_doFoo();
/* 100 more lines of XP cut-n-paste code */
}
OS::_doFoo(...)
{
/* 10 lines of platform-specific code */
}
Again, the change needed on each platform is to delete the code which got
removed to XP land, and wrap the remaining logic inside the new APIs
required. Other than that, is there anything inherently platform-specific
about such a change?
The only way to know that any such refactoring will work for all platforms
is to *look* at all the existing platform implementations, and then actually
remap the code to make sure it can fit inside the proposed new API.
In particular, whoever's doing the refactoring needs to notice that the
implementation for platform 6, say, doesn't fit the pattern described above,
because it actually looks like this:
OS6::doFoo(...)
{
/* 50 lines of XP cut-n-paste code */
/* 10 lines of platform-specific code */
/* 60 more lines of XP cut-n-paste code */
/* 5 lines of platform-specific code */
/* 40 more lines of XP cut-n-paste code */
}
Thus, the XP refactoring actually needed [1] would be more like:
XP::doFoo(...)
{
/* 50 lines of XP cut-n-paste code */
_doFoo1();
/* 60 more lines of XP cut-n-paste code */
_doFoo2();
/* 40 more lines of XP cut-n-paste code */
}
OS::_doFoo1(...)
{
/* 10 lines of platform-specific code */
}
OS::_doFoo2(...)
{
/*
5 lines of platform-specific code on OS6,
and nothing on the other platforms
*/
}
If so, I can't think of any reason why whoever does the XP implementation
shouldn't just do the necessary changes for all platforms, because they
simply can't get the refactoring "right" any other way.
Admittedly, there might be some cut-and-paste errors, but that's what
Tinderbox is for.
>For this we need to provide a static
>constructor for the xp class that is implemented by the platform class.
>So until the platform maintainer implements it, the link will fail.
The hypothetical refactorings I described above didn't use static
constructors, but wouldn't the same principle apply?
Some amount of existing platform code should survive the refactoring, but be
located in new spots. The only *new* code needed on each platform *is* a
cut-and-paste job, and thus should be done once for all platforms at
refactoring time.
What am I missing here?
Paul,
who's made more than his share of tree-wide changes
[1] Note that the variation here *could* be gratuitous, so it might be
worth asking the OS6 maintainer whether all the platform-specific logic
could be called in a single _doFoo() spot. However, from past experience
with this kind of thing, the answer is often "no".
This archive was generated by hypermail 2b25 : Thu Apr 26 2001 - 06:30:18 CDT