woocommerce / woocommerce-google-analytics-integration

WordPress plugin: Provides the integration between WooCommerce and Google Analytics.
http://wordpress.org/plugins/woocommerce-google-analytics-integration/
170 stars 69 forks source link

Make methods that use settings non-static #412

Open tomalec opened 5 months ago

tomalec commented 5 months ago

~User~ developer story

As discussed in https://github.com/woocommerce/woocommerce-google-analytics-integration/pull/398#discussion_r1540147719

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

  1. Turn settings to be instance propery
  2. Turn all methods that use settings directly or indirectly to be non-static
  3. 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

Out of bounds/rabbit holes

Event tracking