Jump to content
puggybelle

How do I use function __construct in this code?

Recommended Posts

I'm very close to running the old SEO-G contrib again, but I'm running into PHP 7 errors.

From my error log:

Quote

[01-Jul-2018 22:33:30 UTC] PHP Deprecated:  Methods with the same name as their class will not be constructors in a future version of PHP; seoURL has a deprecated constructor in /home/myusername/public_html/includes/classes/seo_url.php on line 24

Here's the code:

class seoURL {
    var $path, $query, $params_array, $error_level, $handler_flag;

    function seoURL() {
      $this->path = $this->query = '';
      $this->params_array = array();
      $this->query_array = array();
      $this->error_level = 0;
    }

I think I need to change that third line to something like:  function __construct seoURL, but I'm completely lost on what other punctuation needs to be there.

Anyone know how to rewrite this?

- Andrea

Share this post


Link to post
Share on other sites
class seoURL {
    var $path, $query, $params_array, $error_level, $handler_flag;

    function __construct() {
      $this->path = $this->query = '';
      $this->params_array = array();
      $this->query_array = array();
      $this->error_level = 0;
    }

 

Share this post


Link to post
Share on other sites

Thank You, very much, Rainer!

On that note, I am officially running SEO-G urls in my website again.  Whoo!

Off to tell Google to kiss my thousands of 404 error pages goodbye!

A lot of cleaning up code to make it work, but God bless the person who created the SEO-G Admin add-on for later versions of osCommerce.

Without that, it was a horrible mess and I never would have been able to fix it.

- Andrea

Share this post


Link to post
Share on other sites

Congratulations on getting it to work. Though I don't understand what it has to do with 404's. If they are soft 404's, they will continue to exist unless you make a change for them. See the Header Status Handler in that case.

Share this post


Link to post
Share on other sites

They're not soft 404 errors.  Think I said the wrong thing in my previous post.

They're just flat out...page not found errors.  Thousands of them.

I have visited the Google Merchant Console and declared all of them to be 'fixed'. 

Soft 404 is zero.  Not Found was massive.  Nearly 6,000.

I'll revisit that again many times in the coming days, I'm sure. 

Thanks, Jack!  Your advice regarding what to look for in old contribs was more than helpful!

- Andrea

Share this post


Link to post
Share on other sites

While going through the Frozen code with an eye towards upgrading it to PHP 7.2, I found a number of places where the constructor with the class name was left in place, and a new __construct call was added to make PHP 7 happy.  Something like
 

class Glotz {

    // constructor
    function __construct () {
        Glotz();
    }

    function Glotz () {
...

(or similar). Is this wrapper-style __construct required, or can I simply rename function Glotz to function __construct? I would guess that in the old code, any use of Glotz() would create a new object, rather than just being a vanilla function call (once the object was created), but I'm not quite sure whether that's 100% guaranteed. Does anyone know for sure whether a class constructor (with the same name as the class) can simply be renamed to __construct, or should I play it safe and use the wrapper-style (possibly preserving the availability of a function by that name)?


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites

Well, the point is, can anyone call function "Glotz" without invoking it as a constructor? That is, once an object (of class "Glotz") is created, is it valid to invoke function "Glotz" as an ordinary method (i.e., not creating a new object)? If so, I can see requiring keeping the method around under that name.

Edit: Moot point, at least in the cases where I changed function Glotz() to function __construct() -- those classes apparently aren't used, anyway. I'll still change them, just to be perfectly legal, but I'm still wondering about keeping an ordinary method with the same name as the class (plus, __construct constructor).


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites

I think the code should be

class Glotz {

    // constructor
    function __construct () {

There's no reason to have a function by the class name that I can see. If code elsewhere in the shop calls the function named Glotz, I would change that to a unique name (in the class too).

Share this post


Link to post
Share on other sites
Posted (edited)

I remember a thread where this was discussed for the PHP7 update of EDGE with @BrockleyJohn but can't find it any more.

He developed and tested the class constructer name updates in EDGE. Maybe he can explain the reasons better.

I only remember that without this trick (there are also just empty class constructors used in some classes) function not found errors were thrown and his solution solved them.

Edited by raiwa

Share this post


Link to post
Share on other sites
21 hours ago, MrPhil said:

While going through the Frozen code with an eye towards upgrading it to PHP 7.2, I found a number of places where the constructor with the class name was left in place

Can you show an example please.  Class constructors and evaluation order changes were performed prior to release of Frozen. 

IIRC the Paypal App was rolled back (to make it updateable from within admin) so there may be some in there => awaiting official update.


This is a signature that appears on all my posts.  
IF YOU MAKE A POST REQUESTING HELP...please state the exact version
of osCommerce that you are using. THANKS

 
Get the latest current code (community-supported responsive 2.3.4.1BS Edge) here

 

Share this post


Link to post
Share on other sites

They are  admin/includes/classes/payment_module_info.php (class paymentModuleInfo) and includes/apps/paypal/cfg_params/transactions_order_status_id.php (class OSCOM_PayPal_Cfg_transactions_order_status_id). Neither class seems to be invoked, so no harm done (yet). I don't know if it's better to add a dummy __construct(), or change the constructor name of these two.


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites
Posted (edited)
59 minutes ago, MrPhil said:

They are  admin/includes/classes/payment_module_info.php (class paymentModuleInfo) and includes/apps/paypal/cfg_params/transactions_order_status_id.php (class OSCOM_PayPal_Cfg_transactions_order_status_id). Neither class seems to be invoked, so no harm done (yet). I don't know if it's better to add a dummy __construct(), or change the constructor name of these two.

I only had to do the includes/apps/paypal/cfg_params/transactions_order_status_id.php using __construct()

Edited by 241

No longer giving free advice. Please place deposit in meter slot provided.  Individual: [=] SME: [==] Corporation: [===]
If deposit does not fit one of the slots provided then you are asking too much! :P


Support The Project
Documentation/Knowledgebase/Discussions
 

Share this post


Link to post
Share on other sites
On 5/15/2019 at 9:20 PM, MrPhil said:

Well, the point is, can anyone call function "Glotz" without invoking it as a constructor? That is, once an object (of class "Glotz") is created, is it valid to invoke function "Glotz" as an ordinary method (i.e., not creating a new object)? If so, I can see requiring keeping the method around under that name.

Edit: Moot point, at least in the cases where I changed function Glotz() to function __construct() -- those classes apparently aren't used, anyway. I'll still change them, just to be perfectly legal, but I'm still wondering about keeping an ordinary method with the same name as the class (plus, __construct constructor).

Fom php7, to avoid throwing the deprecated notice, you can only have the method "classname" if you also have the method "__construct".

The equivalent of php4 constructors is to implement method __construct and coding it to invoke method classname with the same parameters. This means you can call it explicitly if you like later on.

However this is a bad idea as the global approach as it's a lot of extra code for no purpose and it masks the very few cases where it's actually required. A better general approach is to change the name of the constructor from classname to __construct and test it from all the places it's used. It's only needed where the method is used to reinitialise an already instantiated object (maybe object_info.php, it's been a while).

I didn't implement any changes that couldn't be tested, so classes that are no longer used were not php7-proofed but do not throw errors in end-to-end osc testing. There are a very small number of these. If I recall the discussion correctly they were left in in case they are used in addons. In my view, the fix in these particular cases is to delete the class file altogether. It'll save a couple of k diskspace and a small maintenance overhead.


For a new install or if your store isn't mobile-friendly, get the community-supported responsive osCommerce (2.3.4.1 CE) here: https://github.com/gburton/Responsive-osCommerce/archive/2341-Frozen.zip

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Share this post


Link to post
Share on other sites

I've noticed a few instances where class Glotz has a function Glotz() and an empty (dummy) __construct(). I'm wondering if the dummy __construct was a mistake -- now no code is being run when the object is created.


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites
17 hours ago, MrPhil said:

I've noticed a few instances where class Glotz has a function Glotz() and an empty (dummy) __construct(). I'm wondering if the dummy __construct was a mistake -- now no code is being run when the object is created.

Can you find them again?


For a new install or if your store isn't mobile-friendly, get the community-supported responsive osCommerce (2.3.4.1 CE) here: https://github.com/gburton/Responsive-osCommerce/archive/2341-Frozen.zip

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Share this post


Link to post
Share on other sites
6 hours ago, BrockleyJohn said:

Can you find them again?

admin/includes/classes/shopping_cart.php

admin/includes/classes/table_block.php

includes/classes/alertbox.php

Do any of these look like trouble?  I.e., a dummy __construct() added to silence the PHP 7 deprecated warnings, but should either have been function classname() renamed to __construct, or __construct() calling classname()?


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites
7 hours ago, MrPhil said:

admin/includes/classes/shopping_cart.php

admin/includes/classes/table_block.php

includes/classes/alertbox.php

Do any of these look like trouble?  I.e., a dummy __construct() added to silence the PHP 7 deprecated warnings, but should either have been function classname() renamed to __construct, or __construct() calling classname()?

Nope these are all deliberate. Shopping cart's eponymous method is called to reset it (but there's no point in resetting it on construction). Table block and altert box's methods are called from child classes which override the constructor so the parent's never gets called.


For a new install or if your store isn't mobile-friendly, get the community-supported responsive osCommerce (2.3.4.1 CE) here: https://github.com/gburton/Responsive-osCommerce/archive/2341-Frozen.zip

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Share this post


Link to post
Share on other sites

OK, good to know that nothing there needs fixing. Although I do wonder if they behaved differently if and when they had only the eponymous constructor methods.


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites
6 hours ago, MrPhil said:

OK, good to know that nothing there needs fixing. Although I do wonder if they behaved differently if and when they had only the eponymous constructor methods.

Whether they would have behaved differently in the past with the current constructors, I couldn't say. However the current core code doesn't - that's what testing is for!

I am confident in the testing of the original set of changes - the process used for the changes was:

  • code scan for all classes and identify those with php4 constructors, code scan for direct calls to constructor method
  • identify at least one page using the class and check for deprecated message
  • show that change removed deprecated message but leaves page function unchanged
  • for direct calls to constructor method, show function before and after the same too

However the changes were reapplied several times over the course of months as Edge moved forward and although largely automated by git, it's always possible that errors or omissions were introduced in the final version so it's good to air any doubts you have.

The constructors resulted in a lot of lines of code that needed altering but the bit that was much harder to analyse confidently was the change to the evaluation order in statements. IIRC there was only one line of code that needed changing for this.


For a new install or if your store isn't mobile-friendly, get the community-supported responsive osCommerce (2.3.4.1 CE) here: https://github.com/gburton/Responsive-osCommerce/archive/2341-Frozen.zip

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×