@mnapoli opened this Pull Request on October 15th 2014 Member

Extracted Piwik::getJavascriptCode() into a separate, non static class: Piwik\Tracker\TrackerCodeGenerator.

The content of the methods hasn't been modified, I have just moved them.

Why?

  • clear up the almighty Piwik class which has too many responsibilities
  • a class with non-static methods means it's open to dependency injection
  • tests are in a separate class dedicated to this new class, so that's also simpler/cleaner

Piwik::getJavascriptCode() is now deprecated in favor of the new class (I have changed the places where it was called). I added the deprecation to the changelog.

I haven't renamed the Piwik.getJavascriptCode event for backward compatibility.

Feedback welcome.

@czolnowski commented on October 15th 2014 Contributor

Hi @mnapoli!
I was thinking about this method and way to refactor them. Clearly the first step is extraction into separate class. Great job! I think that next step might be extraction of all classes and helpers of tracking code into separate plugin. There are dependencies in CoreHome, SitesManager and Morpheus. All of them should be moved to separate plugin.

Clearing up almighty Piwik class is important! :+1

@mnapoli commented on October 15th 2014 Member

As pointed by @tsteur it wasn't even an API method (Piwik\Plugins\SitesManager\API::getJavascriptTag() is the API method) so I removed the old method from the Piwik class, there is no need for deprecation. Even better!

Waiting for the tests to pass before merge.

This Pull Request was closed on October 16th 2014
Powered by GitHub Issue Mirror