PathBuilder-based replacement for ImageProcessingListener::imagePath()

[robertpustulka]

Added VersionUtils class - PathBuilder-based replacement for ImageProcessingListener::imagePath()

[burzum]

I dislike the fact that here are two static classes introduced, usually I try to keep their number very low. Nor I'm sure about the chosen architectural path in this case. I'll need some time to review this in detail.

[robertpustulka]

I also dislike static classes, but I assumed that since some part of the plugin depends on static classes, two more of them wouldn't do any harm :).

I could refactor them to normal classes though.

[burzum]

There should be only StorageManager and StorageUtils. Last one is actually a collection of methods that don't depend on other methods but needed a "home".

But like I said besides them being static I'm not sure I agree to the way it's done. I'll review this as I find the time.

[robertpustulka]

OK then. I wanted to do this as much reusable as possible using some factory pattern. Events (current implementation) don't seem like a good way to retrieve this kind of data, so I was lookig for a different approach.

[burzum]

How are we going to continue with that change?

Honestly, I think most of it can stay as it was before. I still don't like the idea of giving up the event in favor of a static class. I agree the version stuff could be somehow improved, I'm just not yet sure on this either.

My hotels internet connection SUCKS big time and isn't available most of the time... So I probably won't find the time to work on this late night. Next week I might find time, I have vacations but I we've planed to do things apart from sitting in front of a computer. :)

[robertpustulka]

I think that it would be no problem to convert static class into an instantiable class. I'd prefer some factory pattern here instead of observer pattern implemented in a place where's nothing to observe :)

[robertpustulka]

The more I think about it the more I'm convinced that it wasn't the best choice to do file processing in classes so thightly coupled to the events system. The observer pattern is ok for hooking into ORM events, but I'd see it only as a proxy for some more flexible system, which could be accessed directly not having to rely on events.

I think I'm just going to port the old imagePath callback to ImageProcessingTrait.
Then I can start thinking how to refactor the code to make it more flexible so events could be by-passed where they are not nessessary. But that's for like 3.2 version of the plugin :)

[robertpustulka]

I'm closing this and I'm gonna open another PR.

[robertpustulka]

No new PR, but a few commits in #74 which has grown too big and should be tested, and merged ASAP as more ideas start to depend on changes made there...