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

>> I don't think Type is a good name, Document would probably be better (if >> I >> understand what it does right: storing the document (script or library) >> that will be transformed by a strategy). > > <Type>, as, without any doubts, you will have seen in the source, is the > "abstract > concept" of what _has_to_be_created_ and only by chance holds the > "original > document" to be processed. It's more the notion of what the output will > be: > "document" of <Type> script or of <Type> library, two different > implementations > of said concept. 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). I still think that this class should not hold the strategies (or the decorators). > What about the name <Code>, which, IMHO, does reflect even better what is > being looked at - with "Document" I do associate more something to deal > with a > word processor, a grafical tool etc. Just a thought. 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? > I do follow you on this, but the reasoning behind the separation of > stratagies and > decorators is very simple: > > - Strategies are _mandatory_, i.e. it does not make sense to create a > "Document" > object without knowing in advance, which <Strategy> to apply (remember, > that > the context of ScriptReorganizer is the deployment of code and not a > dynamic web > application - it's just a work horse package) ... it simply can't be > missed by chance > and it's anchored with the instantiation of the "Document". 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). Unrelated: I had a look to you library type, wouldn't you have a problem with cyclic imports (like A.php contains require_once 'B.php'; and B.php contains require_once 'A.php';)? -- Vincent