Re: [PEPr] Comment on Tools and Utilities::ScriptReorganizer

On Tue, 17 May 2005 11:53:08 +0200, Vincent Lascaux <vincent.lascaux@centraliens.net> wrote:

OK... Looking at the code, it looks like Type is more like an abstract concept of a job: its role is not only to represent some code, but also the way it will be transformed (since it also stores the strategy and (eventually) the decorator).

No Vincent, <Type> does not store any <Decorator>s! <Type> is not aware of the fact that there is any further processing involved _before_ or _after_ the <Strategy> to apply. That's the nice thing about decorators ;-) Furthermore, the entity dealing with a <Type> object does not even know which it is currently using or if it is "decorated". Nice.

Oh, OK, my bad... I like this design of Decorators. Why don't you use the same one for Strategies? Again I'm not sure about the distinction you make between decorators and strategies. You said it is based on the fact that decorators are optional and strategies are mandatory... In my opinion, strategies should also be optional.

For the reason that the main goal (responsibility) of ScriptReorganizer is to reduce (strategy) the file's size to deploy, so to promote the best practice of a two-way deployment. Why should the main goal be optional? The design is geared towards that end. In this context the devoloper who wants to deploy code will ask himself the following questions: what <Type> of optimized file do I want to deliver as a service to the client? How should the source code be optimized? IMHO both questions are linked together "atomically" and thus strategies are mandatory. Just my point of view, as I understand this specific problem domain.

I still think that this class should not hold the strategies (or the decorators).

According to the Gang of Four, the Strategy pattern's intent is to: Define a family of algorithms, encapsulate each one, and make them interchangeable. Strategy lets the algorithm vary independently from clients that use it.

OK, if you want to keep this design, I think you should have an "empty" strategy that does nothing, and be able to combine strategies (like you can combine decorators).

The user of the package is enabled to add a (user) strategy <Empty> at any point in time to the strategy class hierachy and only to use (self developed) decorators, if it fits his purpose, although it does not make sense to me in the context I have designed ScriptReorganizer. See my arguments stated above.

I also prefer Code: it's a more specific (and thus better) name than Document. But a Code should not hold a decorator or a strategy, should it?

Why not? <Code> would only be the conceptual view of <Script> and <Type>. See my notes above.

Because when I'm writing some code I don't think about the way I will remove the whitespaces or the comments afterward. A definition for code would be something like "some data that can be executed by a machine", not "some data and a way to transform it to a more compact but equivalent data that can be executed by a machine". I think Code is a good class name anyway...

Correct you are. As a developer I should not be bothered with the topic of how the source could be optimized and of how to have to code the library/application accordingly. I want to deliver Good Looking Code (TM) without any constraints. One side of the coin. From the ScriptReorganizer perspective however, the Type/Code/Document object has the responsibility of reducing the file's size on the source level and it does not care of how the code is structured. The other side of the coin. Please separate these two perspectives in your thinking and do not try to approach this proposal from "both sides of the river" at the same time.

Actually, I find you library class (that implements Type) usefull, even without applying any strategy. Strategies should not be mandatory, and it would be nice if several strategies could be applied (like 1-remove comments, 2-remove unusefull whitespaces, 3-add header).

Please look at the source of the strategies <Route>, <Quiet> and <Pack> as hinted in the proposal: they are working in an incremental way. <Pack> (allows only one whitespace between each token) depends on <Quiet> (stripping off comments) depends on <Route> (stripping off only two or more consecutive blank lines). So there's no need to use several strategies in combination.

Yes, I had a look to those strategies, and I find it would be more usefull if we could not have this dependency...

The _core_ strategies have been implemented following the "Once and only once" rule: due to the fact that ScriptReorganizer offers an incremental way of reducing the file size, why should i have to code the same (partial) algorithm over and over again? I'm just re-using what showed up as proved/tested. The coupling of the strategies is, IMHO, acceptable in the context they are used. Should new strategies be added, they might not need to rely on the one already existing. Who knows? However, having said this, I'm prepared to remove said dependencies should the package be accepted into PEAR and positive votes being conditional in this respect.

Say I write a new strategy that renames variable to shorter names (blabla => a, foobar => b...). Now I may want to apply this strategy, but leave the formatting of the code unchanged (because I want to check everything is OK, I want to compare old to new code or whatever). I also may want to apply this strategy and the Route strategy, or this one and the Quiet one, or this one and the Pack one. This could be done by having a constructor that lets me create a RenameVariables strategy with another strategy (new RenameVariables(new Quiet) for example).

I think that we are running in circles. With the design proposed this can be achieved without any hassles: $library = new ScriptReorganizer_Type_Decorator_RenameVariables(

    new ScriptReorganizer_Type_Library ( new ScriptReorganizer_Strategy_Empty )

); Here RenameVariables (or if you like: Obfuscate) is clearly an additional and optional functionality (decorator) one would like to add on top of the _core_ reorganization of the source. The possibilities of chaining are endless ...

Or this could also be done by changing the way a strategy is applied to a code: $code->apply(new RenameVariables()); $code->apply(new Quiet());

Please be so kind to enlighten me regarding the following: at the stage of instantiating a <Script> or <Library> object the _core_ <Strategy> to use should already be known too, am I right? So where is the need of deferring the "definiton" of the stratagy to apply? On the other hand, if you are talking about additional functionalities (decorators), these can be added at any time, if needed and according to any "business rules" to follow: $library = ...; // as stated in the example above // here follow some checks regarding external dynamic constraints // which can't be known in advance ... if ( 'external event' == $condition ) {

    $library = new ScriptReorganizer_Type_Decorator_<Decorator 1>( $library );

} else {

    $library = new ScriptReorganizer_Type_Decorator_<Decorator 2>( $library );

} // continue the processing ... So your idea is just a different approach to the same problem.

$code = new ScriptReorganizer_Type_Script( new ScriptReorganizer_Strategy_Route ); $code->load( 'scriptToLoad.php' ); $script = new ScriptReorganizer_Type_Decorator_AddHeader( $code ); $script->reformat( 'HEADER' ); $script = new ScriptReorganizer_Type_Decorator_AddFooter( $code ); $script->reformat( 'FOOTER' ); $script->save( 'fileToSave.php' ); Somehow cumbersome, but would that be a good compromize for you?

If the 'Header' of the reformat function was given as an argument to the constructor, I could have written $code = new ScriptReorganizer_Type_Decorator_AddHeader('HEADER',

    new ScriptReorganizer_Type_Decorator_AddFooter('FOOTER',
        new ScriptReorganizer_Type_Script(
            new ScriptReorganizer_Strategy_Route
        )
    )

); $code->load('scriptToLoad.php');

// first alternative $code->reformat(); // the default HEADER and FOOTER will be added // second alternative $code->reformat( 'HEADER 2' ); // the overriding HEADER will be added

                                // as well as the default FOOTER

$script->save('fileToSave.php');

Vincent, you can do this, for this feature is already implemented in the <AddXXX> decorators!: (1) either you can add a default header/footer via the constructor (2) or you can programmatically add an overriding header/footer with the reformat method. See e.g. the http://pear.sf.rausch-e.net/pepr/ScriptReorganizer/docs/index.html for the <AddFooter> decorator. You have the choice, which approach suits you best. The only constraint with the implementation of these decorators is that you can't add an overriding header _and_ footer - at least for the time being. I will address this issue, if there's the need for it in the future.