User developer story
As discussed in #398 (comment)
We use a lot of static functions that are not 100% "static," IMO.
PHP spec
Declaring class properties or methods as static makes them accessible without needing an instantiation of the class.
So we could change self::get( * ) to $this->get() or even $this->settings[ * ] (IMHO, get just blurs the picture here w/o much benefit).
Is your feature request related to a problem?
It's not a big problem. This is a singleton, so the line between class and instance is blurred anyway. However, the static could mislead developers by suggesting no instance needs to be created to use them, which is not true.
How to reproduce the problem
Use WC_Google_Gtag_JS::get without creating the instance first.
Describe the solution you'd like
- Turn
settings to be instance propery
- Turn all methods that use
settings directly or indirectly to be non-static
- Remove
get function, as currently it limits the data types to be used as settings to string which may be problematic in the future
Describe alternatives you've considered
Do only 1-2 above.
Technical
Figma link
Acceptance criteria
Unknowns
- There may be some limitations, logic, or convention put around
settings and get in the WC Core class WC_Integration
Out of bounds/rabbit holes
Event tracking
Userdeveloper storyAs discussed in #398 (comment)
We use a lot of static functions that are not 100% "static," IMO.
PHP spec
public static function get_enabled_eventsusesself::getpublic static function load_opt_outusesself::getpublic static function get_product_identifierusesself::getpublic static function getuses theself::settingsbut we set the
self::settingsin the instance constructor https://github.com/woocommerce/woocommerce-google-analytics-integration/blob/cdb60c08a9cb62f351f5d324472ad62b6eef7105/includes/class-wc-google-gtag-js.php#L47:L47So we could change
self::get( * )to$this->get()or even$this->settings[ * ](IMHO,getjust blurs the picture here w/o much benefit).Is your feature request related to a problem?
It's not a big problem. This is a singleton, so the line between class and instance is blurred anyway. However, the
staticcould mislead developers by suggesting no instance needs to be created to use them, which is not true.How to reproduce the problem
Use
WC_Google_Gtag_JS::getwithout creating the instance first.Describe the solution you'd like
settingsto be instance properysettingsdirectly or indirectly to be non-staticgetfunction, as currently it limits the data types to be used as settings tostringwhich may be problematic in the futureDescribe alternatives you've considered
Do only 1-2 above.
Technical
Figma link
Acceptance criteria
Unknowns
settingsandgetin the WC Core classWC_IntegrationOut of bounds/rabbit holes
Event tracking