Decoupling of merge/minify functionality

[marcovtwout]

I love the original idea of this extension, but I think it has become too bloated by the integration of merging and minifying parts. Just look at the list of options (https://github.com/nlac/nlsclientscript/blob/master/src/nlac/NLSClientScript.php) vs version 5, for example.

I suggest all this extra functionality to be made truly optional and decoupled from the main NLSClientScript class. Coupling could be done similar to how Yii handles coupling Pagination to a DataProvider: you can pass options to instantiate the Pagination object, or you can simply set the whole "object" to false: http://www.yiiframework.com/doc/api/1.1/CDataProvider#pagination-detail

What do you think about that?

[nlac]

hi, i agree and already took steps towards that (maybe you noticed that many functionalities have decoupled from the "core" in 7.0, the size of the main file decreased from 25k -> 11k compared with v6.7 :-) It's also planned to create a separate repository for the css merger as pointed on http://nlacsoft.net/nlsclientscript/merging-css. So if i understand correctly, you suggest only to re-arrange the configuration object into the style you mentioned, or maybe also other type of decoupling in the code?

[marcovtwout]

It's not just about the configuration object. If you look at the above example, you can see that CDataProvider only has a getter and setter for the pagination object, and CPagination contains the actual implementation (as well as the configuration).

While you're at it, you should also consider implementing the dependency injection pattern (http://www.phptherightway.com/#dependency_injection). For example, if you want to extend/replace your NLSDownloader, you currently would have to subclass the NLSClientScript class as well (https://github.com/nlac/nlsclientscript/blob/master/src/nlac/NLSClientScript.php#L198). There are multiple ways to go about this, as long as you provide a way to inject the dependent class into the given class. Yii core code has plenty of examples too.

And while I've seen you already separated a few classes, there is still combined functionality in the main class (like https://github.com/nlac/nlsclientscript/blob/master/src/nlac/NLSClientScript.php#L243 and https://github.com/nlac/nlsclientscript/blob/master/src/nlac/NLSClientScript.php#L320).

Keep at it! :)

[nlac]

ok i think i get it.. the first part (enabling to pass the class of the sub components) is easy, the second (decoupling the full merge functionality) requires more considerations and work. I'll think about it, and keep that one open.