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)?

Share this post


Link to post
Share on other sites

It is required where done like this. Otherwise the function "Glotz" if called from outside will not be found, just like you say.

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).

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 Responsive osCommerce CE (community edition) 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.

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 (Phoenix).

here: on the official osc download page

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.

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 (Phoenix).

here: on the official osc download page

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()?

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 (Phoenix).

here: on the official osc download page

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.

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 (Phoenix).

here: on the official osc download page

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

Hi,
With an Oscommerce version 2.3.4 with many own add-ons, I had the same problem during the upgrade to the latest state of PHP 7.
For example, I received this error message in the "search function" when searching for "keyword)". Note the ")" was a typo.
Example of the errorlog:
[29-May-2019 15:55:30 Europe / Amsterdam] PHP Fatal error: Uncaught Error: Call to undefined method message Stack :: tableBox () in / home / xxxxx / domains / my-websitename / public_html / includes / classes / message_stack.php: 74
Stack trace:
# 0 /home/xxxxx/domains/my-websitename/public_html/advanced_search.php(114): messageStack-> output ('search')
# 1 {main}
  thrown in /home/xxxxx/domains/my-websitename/public_html/includes/classes/message_stack.php on line 74
I was able to solve this problem through the instruction of dr_Lucas (Michaela). You can find this via the link: https://forums.oscommerce.com/topic/408220-php-7/?tab=comments#comment-1739026
I hope it will also help you solve this problem.
There are more classes where the "extend" is used, so you will also have to correct it.

Share this post


Link to post
Share on other sites

@LuckyPiedro yes, as described by @MrPhil above this is one of the three classes in responsive osc where you need to add an empty menthod

function __construct() {
}

instead of renaming the constructor. You should make sure that you have done the other two properly as well or eventually you'll hit other errors. There are likely others for 2.3.x unresponsive but I didn't evaluate every single one.

PLEASE NOTE - you do not need to do any of these constructor changes to your store. You can simply set reporting not to show deprecated warnings and there is no issue with this on php7. The version at which support for php4-style class constructors will be removed altogether has yet to be confirmed but it was proposed to be php8. If you are going to invest time in updating your store code, it would be much better spent on making the store responsive which will actually increase your traffic. Of course, if you are running an old version of osc with a responsive skin then you've already done that!


For a new install or if your store isn't mobile-friendly, get the community-supported responsive osCommerce (Phoenix).

here: on the official osc download page

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

Hi john,
Thank you for your comment. I am fairly new to this world and thought I was making a contribution as I have already used it several times with other contributions. I understand that you can turn off the notifications but as an interim solution I am in favor of viewing the notifications and resolving them for the current version. Of course it is better to install the responsive version but that naturally requires a little more time and that is exactly the point. It is not always there.

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

×