Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static invocations such as __::each raise E_STRICT warnings #4

Open
jdp opened this issue Dec 10, 2011 · 18 comments
Open

Static invocations such as __::each raise E_STRICT warnings #4

jdp opened this issue Dec 10, 2011 · 18 comments

Comments

@jdp
Copy link

jdp commented Dec 10, 2011

Using the static method invocations, like __::each and __::map will raise E_STRICT warnings. For example, calling __::filter will raise this chain of warnings:

Array
(
    [number] => 2048
    [string] => Non-static method __::filter() should not be called statically
    [line] => 55
)
Array
(
    [number] => 2048
    [string] => Non-static method __::select() should not be called statically
    [file] => ../lib/underscore.php
    [line] => 182
)
Array
(
    [number] => 2048
    [string] => Non-static method __::_wrapArgs() should not be called statically
    [file] => ../lib/underscore.php
    [line] => 184
)
Array
(
    [number] => 2048
    [string] => Non-static method __::_collection() should not be called statically
    [file] => ../lib/underscore.php
    [line] => 186
)

As of PHP 5.4, E_STRICT is or'd into the E_ALL constant, so scripts that report all errors will show these.

@brianhaveri
Copy link
Owner

Thanks for putting this on the radar. This issue ties back to supporting both static calls and OO-style calls.

Having looked into a solution before, nothing jumped out as particularly clean or easy. I'll take a look and see what I can do. Having E_ALL report these in 5.4 makes this more important than in previous versions.

@frosas
Copy link

frosas commented Mar 7, 2012

What about something like this?

<?php

function __() {
    return Underscore::instance();
}

class __ {

    static function __callStatic($name, $args) {
        return call_user_func_array(array(Underscore::instance(), $name), $args);
    }
}

class Underscore {

    static function instance() {
        static $instance;
        if (! $instance) $instance = new self;
        return $instance;
    }

    function foo() {
        return 'foo';
    }
}

__()->foo();
__::foo();

@brianhaveri
Copy link
Owner

Something like that might work. I briefly tried this approach, but only a couple of tests passed. It's probably worth a second look.

function __($item=null) {
  $__ = Underscore::getInstance();
  if(func_num_args() > 0) $__->_wrapped = $item;
  return $__;
}

class __ {
  public static function __callStatic($method, $args) {
    $__ = Underscore::getInstance();
    if(count($args) > 0) $__->_wrapped = $args[0];
    return call_user_func_array(array($__, $method), array_slice($args, 1));
  }
}

class Underscore {
  // All the functions go here
}

@joseym
Copy link

joseym commented Apr 22, 2012

Any progress on resolving these warnings?

@ryanve
Copy link

ryanve commented May 6, 2012

@frosas @brianhaveri I've been pondering this issue too. I like the idea of using overloading. I think it'd be best to do it in reverse (using __call rather than __callStatic):

<?php
function __2($item=null) {
    return new Underscore2($item);
}

class __2 {
    public static function foo() {
        return 'foo';
    }
    // Mark all fns as "public static" to avoid the "calling non-static 
    // methods statically generates an E_STRICT" warning described on
    // php.net/manual/en/language.oop5.static.php
}

class Underscore2 {
    private $_wrapped;

    public function __construct($item = null) {
        $this->_wrapped = $item;
    }

    public function __call($name, $args) {
        array_unshift($args, $this->_wrapped);
        $this->_wrapped = call_user_func_array('__2::' . $name, $args);
        return $this->_wrapped; // add chain logic here to determine if chained
    }
}

I was curious about how much overhead the overloading added and did some tests here.
@brianhaveri Mad props on this project—it's pretty sick! =]

Edit - I took out the extends. In that case the the overloader __call is not triggered because the inheritance happens and the method are available. That would have been great tho. In any case my guess is that the static __:: syntax is the one that more people use so it makes sense to design for that.

@joseym
Copy link

joseym commented May 6, 2012

@ryanve very elegant solution!

@ryanve
Copy link

ryanve commented May 7, 2012

@joseym Thanks - although not quite as good as how I originally thought. See my edit above. It's basically the reverse of what @frosas has.

@frosas
Copy link

frosas commented May 7, 2012

My code above was intended as a quick fix for the current implementation. As procedural style is probably the most used I also prefer @ryanve approach and avoid instantiating objects if possible to avoid performance issues.

@bayleedev
Copy link

A property declared as static can not be accessed with an instantiated class object (though a static method can).

My best suggestion is to make a __static class which has ONLY static methods. This can be used statically like you'd expect. Then you'd have a __ like expected which has a __call and __callStatic which can wrap it as expected. The issue with this is the current library cannot be used statically, and doing the static library with an init and static library has a few side effects. A complete refactor is in the near future of this library.

Concept Draft

@joseym
Copy link

joseym commented Jun 29, 2012

@BlaineSch I think we all understand the issue, I have yet to try @ryanve workaround but it looks promising and doesn't require a rewrite of the entire library.

@bayleedev
Copy link

@joseym The wrapper class I made is similar to his approach, the problem with making the entire library static was the instance variables will no longer be accessible, and making those static is obviously a bad idea. So the reverse approach is needed, which is how I implemented the draft I linked to above. The wrapper handles static and non-static requests and basically always has an instance of the library.

I realize the issue was known, I was just walking through it in my head.

@oojacoboo
Copy link

Why isn't BlaineSch's solution implemented in this lib?

@bayleedev
Copy link

@oojacoboo Judging by the activity, the project is dead. The approach is backwards and would probably need a rewrite. There should be an object with a static class backend that contains the logic.

Check out this similar array library I created.

@oojacoboo
Copy link

@BlaineSch yea... looks pretty dead. There is another lib I found on github, but it has some pretty nutty Laravel dependencies and such. It also uses a config file for mapping :/

@JonathanAquino
Copy link

I created a fork that eliminates the E_STRICT warnings by removing the object-oriented way of calling the functions, and changing all the functions to static: https://github.com/JonathanAquino/Underscore.php .

@yankeeinlondon
Copy link

Great Jonathan! Looking forward to trying it out. Can I assume that the
functional footprint and signatures are the same? Might also be worth
pointing people to Brian's documentation as this is an important reference
item: http://brianhaveri.github.io/Underscore.php/.

Ken

On 20 September 2013 22:46, Jonathan Aquino [email protected]:

I created a fork that eliminates the E_STRICT warnings by removing the
object-oriented way of calling the functions, and changing all the
functions to static: https://github.com/JonathanAquino/Underscore.php .


Reply to this email directly or view it on GitHubhttps://github.com//issues/4#issuecomment-24843315
.

@JonathanAquino
Copy link

Hi Ken: Yeah, the signatures are the same, although I had to remove the chain() method. Also, good idea about adding a link to Brian's doc - I have done that.

@dvapelnik
Copy link

up!
I'm trying to use this lib from https://packagist.org/packages/underscore/underscore.php with php-v5.3.3-7 and caught warning and error:

  1. Warning like Non-static method __::filter() should not be called statically
  2. Error: Call to undefined function __()

@bayleedev bayleedev mentioned this issue Jun 29, 2012
TalonTR added a commit to InSitu-Software/Underscore.php that referenced this issue May 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants