Converter refactoring
Lars Strojny
while digging through the code coverage report this day I found out that
those parts that are not well tested in PHP IDS are mostly converters
and especially those harder to test issues which are dependent on PHP
versions, or specific extensions being enabled. The testing of it is
even made harder because we suffer from static cling in our converters.
Our converters are a big ball of mud - all is stuffed into our single
IDS_Converter class.
So I've read through the code and found an elegant way to refactor
converters: in the proposed design every converter resides in its own
class. We basically have two types of converters: those who are executed
every time and those who depend on a certain pre condition. This
precondition can be a specific state of the passed value or a more
generic state like a PHP version.
Every converter implements the interface IDS_Converter_Interface, which
defined a simple convert()-method. The good thing is, convert() takes a
reference to the value so that it really converts, not copies.
The more advanced interface for so called conditional converters (those
depending on a PHP version or specific state) is
IDS_Converter_Conditional. This interface extends the basic converter
interface and adds a method: enabled(). This method takes one parameter:
the value and returns a boolean.
These scheme makes it even possible to refactor more complex converters
like the charcode converter into a few simpler once.
I have a fully working prototype in a branch[1] and the relavant package
can be found here[2] and a wiki document describing what has been done
and which IDS_Convert-method resulted in which converter object[3].
Although the branch is working, some things are still missing:
a) Document comments
Branch [1]: https://trac.php-ids.org/index.fcgi/browser/branches/converter-refactoring
The new IDS_Converter package [2]: https://trac.php-ids.org/index.fcgi/browser/branches/converter-refactoring/lib/IDS/Converter
Wiki entry [3]: https://trac.php-ids.org/index.fcgi/wiki/Refactoring/Converter
cu, Lars
Lars Strojny
I pressed "send" to early, will just go on inline.
Am Dienstag, den 23.12.2008, 01:24 +0100 schrieb Lars Strojny:
[...]
> Although the branch is working, some things are still missing:
> a) Document comments
b) Refactor internal structure of the converters for symmetry and
intuitive code [4]
c) Decide on the "enabled" naming. I don't have a better one, but I'm
sure you'll have
d) Write unit tests for each converter
e) Write unit tests for the more complex converters like
IDS_Convert_Encoding_Utf7_*
The Comments and InlineComments converters are in a good shape:
https://trac.php-ids.org/index.fcgi/browser/branches/converter-refactoring/lib/IDS/Converter/Html/Comments.php
https://trac.php-ids.org/index.fcgi/browser/branches/converter-refactoring/lib/IDS/Converter/Html/InlineComments.php
I appreciate your comments and suggestions!
cu, Lars