yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

Add static method to return the class name in Object class #172

Closed petrabarus closed 11 years ago

petrabarus commented 11 years ago

One of great hassle when I develop using Yii1.1 is that I have to write the name of the class manually for methods like beginWidget etc. And it's hard to use it using IDE like Netbeans.

I'm proposing to add get_called_class wrapper in Object class http://php.net/manual/en/function.get-called-class.php

Since the minimum PHP version is 5.3.0, I think this is okay.

It will be something like this

   ///Object.php
    /**
     * Returns the class name.
     * @return string the name of the class.
     */
    public static function getClassName()
    {
        return get_called_class();
    }

that way I can easily use this in the view, or somewhere else.

/* @var $this yii\base\View */
$this->widget(app\components\TestWidget::getClassName());

and the IDE can easily recognize the TestWidget class, and I can easily click on it to go to the file.

qiangxue commented 11 years ago

@yiisoft/core-developers what do you think about this? How about naming it klass() or className().

petrabarus commented 11 years ago

if I can choose, I would vote className().

nkostadinov commented 11 years ago

Definetly className().

resurtm commented 11 years ago

Another variant is clazz. Voting for clazz:

  1. Shorter than className.
  2. clazz would be easier to type and autocomplete in IDE in comparison with klass (since first three letters are same as in class word).
Ragazzo commented 11 years ago

am i right that the only thing this is needed is to IDE? :S if it is about some DI than why not php app\components\TestWidget::$className ? also need to consider this https://wiki.php.net/rfc/class_name_scalars

nkostadinov commented 11 years ago

I dont think we are looking for the shortest word or for some fancy-hard-to-guess word. It should be easy to remember or to guess(when you are learning). IMO

petrabarus commented 11 years ago

@Ragazzo the link you posted states

Status: Implemented in PHP 5.5

the minimum version required for Yii2 is 5.3, I don't think we should put that in the core class like yii\base\Object

and how do you propose filling the class name value to the static variable $className?

mdomba commented 11 years ago

className() makes more sense in any case

Ragazzo commented 11 years ago

@petrabarus i dont even know why these even needs in framework? for what? for your IDE autoclomplete, thats all? naahh... If it will be implemented only for reason "for IDE" it is so stupid.

creocoder commented 11 years ago

Guys please only not klass or clazz. Vote for className().

nurcahyo commented 11 years ago

vote for className() too

On Wed, May 8, 2013 at 2:22 PM, Alexander Kochetov <notifications@github.com

wrote:

Guys please only not klass and clazz. Vote for className().

— Reply to this email directly or view it on GitHubhttps://github.com/yiisoft/yii2/issues/172#issuecomment-17590494 .

klimov-paul commented 11 years ago

Creating methods like "className()", "getClassName()" and so on sounds confusing to me. What is the purpose of such method? If it could serve only for the IDE support, I see no use for it.

klimov-paul commented 11 years ago

Also string like: $this->widget(app\components\TestWidget::getClassName()); looks weird.

creocoder commented 11 years ago

@klimov-paul String like:

$this->widget('app\components\TestWidget');

looks much better?

creocoder commented 11 years ago

@klimov-paul Take it just as OOP get_called_class() version. Sure it can be useful not only for IDE needs.

Ragazzo commented 11 years ago

I totally agree with @klimov-paul , not to be implemented. @creocoder does this looks good:

so not simply just type class, lets make it in OOP way... :S

creocoder commented 11 years ago

@Ragazzo As I say this feature useful NOT only for IDE needs. For example you already have object instance. You can use $object::className() instead of get_class($object). Add to this IDE autocomplete, add to this IDE class recognize. All this make feature very useful.

Ragazzo commented 11 years ago

You can use $object::className() instead of get_called_class($object).

not so convinced as it may looks like.

andersonamuller commented 11 years ago
Ragazzo commented 11 years ago

@andersonamuller so all this are IDE features. What do u mean by "Refactor" ?

andersonamuller commented 11 years ago

Refactor is also an IDE feature. But my point is, with so many pro arguments, plus, this method is not obscure neither obstructive. And the only argument against I read here is "looks weird" and "stupid". It doesn't seem so to me: $object::getClassName() Looks natural.

rawtaz commented 11 years ago

If the method is static one probably shouldn't call it on an object instance (which $object seems to be).

Ragazzo commented 11 years ago

@andersonamuller no, the main reason is that this class method need to be added only for IDE, thats all. You can use get_called_class($object) in your use-case. If it will be adopted then other guys from Eclipse, PhpStorm, Sublime, EtcIDE... will say lets implement that good for IDE feature or interface for autocomplete and what will you do? you need to merge it too, because you've merged one before. This is not needed by Yii2 components behavior so dont need it.

creocoder commented 11 years ago

If the method is static one probably shouldn't call it on an object instance (which $object seems to be).

You are not right. $object::staticMethod() works as indeed.

klimov-paul commented 11 years ago

$object::getClassName() Looks natural.

Can not see anything natural, as there is standard functions "get_class()" and "get_called_class" already. What we will wrap next: "foreach", "echo"?

this feature useful NOT only for IDE needs

@creocoder, could you provide more explicit example?

good for IDE feature

This may sounds ugly, but your IDE is your problem.

petrabarus commented 11 years ago

If you read this http://php.net/manual/en/function.get-called-class.php

 Description
string get_called_class ( void )
Gets the name of the class the static method is called in.

it is intended to be called inside a static method of a class.

the difference with get_class is that get_class accepts object.

creocoder commented 11 years ago

@klimov-paul

could you provide more explicit example?

You know that no ;)

Ragazzo commented 11 years ago

This may sounds ugly, but your IDE is your problem.

@klimov-paul yes, thats why it is dont need to be implemented. also as for 5.5 i think it would be in stable by that time, you got some other class resolve methods as i mentioned above.

andersonamuller commented 11 years ago

Too bad we can't do something like


$this->widget(get_class(app\components\TestWidget));
klimov-paul commented 11 years ago

What is bad with the old fasion doc comments like: /* @var $widget TestWidget*/

creocoder commented 11 years ago

I'm not sure, but maybe:

TestWidget::widget();
TestWidget::beginWidget();
TestWidget::endWidget();

good exit from situation, but this lead to change arch a little. No need excess methods and IDE will be happy.

andersonamuller commented 11 years ago

@klimov-paul There is nothing bad with that, my concern is using plain strings to represent a class.

$this->widget('app\components\TestWidget');

But I don't think we can do much about this, since the framework heavily relies on it.

rawtaz commented 11 years ago

@creocoder I used the word "should" because if you are declaring a static function, then you are designing it to be called statically and not as a method.

petrabarus commented 11 years ago

@klimov-paul in fact for every $this->widget or $this->beginWidget call in my project (which occur about thousand calls), now I write

/* @var $widget1 Widget1*/
$this->widget('wWidget1');

/* @var $widget2 Widget2*/
$this->widget('Widget2');

while the $widget1 and the $widget2 are not even there.

creocoder commented 11 years ago

@rawtaz You write:

If the method is static one probably shouldn't call it on an object instance (which $object seems to be).

Check Yii 2 source code to ensure you are not right. $object::staticMethod() very often used. But maybe you mean something else?

rawtaz commented 11 years ago

@creocoder I'm talking about how you write/design the code, not what technically works in PHP - there's a ton of bad things we can do that will work in PHP. Anyway, if what you say is true then the conclusion is that the core devs have decided to utilize this "feature" of PHP (that one can call a static function on an object instance, be it since 5.3 or 5.4, I don't remember). The fact that it works is one thing, whether one consider it clean code is another. I'm not going to try convincing you on this, we're already cluttering up the original discussion.

bwoester commented 11 years ago

@rawtaz I think you should look at static methods from a slightly other angle: If you define a method as being static, you permit it being called without an instance, but you don't limit it to a static context because of this. I usually don't call them via $instance::staticMethod(), but use the very same syntax as for every other method call: $instance->staticMethod(). I don't care if that method has access to my instance's data or not. And I don't want to remember every modifier of every method. AFAIK, this has been possible for a long time and is possible in other languages I use, so I don't think this is exploiting something that just luckily works and might be removed in future versions. It's really how I think it is meant to work. The new thing introduced in PHP 5.3 is the possibility to use $className::staticMethod(). This is different than $instance::staticMethod() and another reason why I would avoid that syntax.

About the original topic: It will help me during development, so I want to have this feature.

samdark commented 11 years ago

@bwoester you'll get a deprecation warning if calling static method non-statically.

bwoester commented 11 years ago

@samdark Sure about that? Don't find anything related on php.net. There are a few related problems like calling non-static methods statically and accessing static attributes on instances which doesn't work. Also the following quick test doesn't show any warnings to me:

class Test
{
  public static function foo() {
    return true;
  }
}

error_reporting(-1);
$t = new Test();
$t->foo();

I'm using PHP 5.3.13 on this machine.

samdark commented 11 years ago

@bwoester no, it's actually the other way around... Still I think it's very confusing to do so.

bwoester commented 11 years ago

I guess whenever there are different ways to do something, there will be different preferences. Nothing wrong with that. :wink:

I'd only double check with that new possibility of $className::staticMethod() vs. $instance::staticMethod(). To make sure it doesn't cause overhead or other side effects.

creocoder commented 11 years ago

@bwoester

There

$className::staticMethod() vs. $instance::staticMethod()

all OK. Yii 2 actively use this feature.

But $instance->staticMethod() is NOT OK, really.

qiangxue commented 11 years ago

@creocoder We are not using $instance::staticMethod(). I don't think this is a valid syntax. $instance->staticMethod() is fine.

qiangxue commented 11 years ago

We are straying too far from this issue. Let's get back to the issue.

When I first saw this issue, my immediately response was also to reject it because I can't stand putting something like this in a base class.

However, I found its value after a while. The value right now mainly lies in the usage of $this->widget() as already explained by @petrabarus.

Since PHP 5.5 is adding $className, I think that's another point why this method is useful.

samdark commented 11 years ago

@qiangxue https://wiki.php.net/rfc/class_name_scalars ?

suralc commented 11 years ago

I'd also like to see this implemented somehow, since it can improve readability of view-files.

If I got the widget mechanism right we have to specify the full classname of that widget. In a deep-structured app, this may lead to namespaces like that: <app-name>\components\<subproject-name>\widgets\frontend\user\profile\DisplayWidget which would look something simliar to this in app-code:

<html>
.......
<?php $this->widget("<app-name>\components\<subproject-name>\widgets\frontend\user\profile\DisplayWidget", array()); ?>

It takes about two seconds to even realize what widget is called there.And another moment to see what it might output.

using the new method the time of recognition might be cut down, leading to an easier to read code which is not so troublesome to maintain:

<?php use <app-name>\components\<subproject-name>\widgets\frontend\user\profile\DisplayWidget as UserProfileWidget; ?>
<html>
.....

<?php $this->widget(UserProfileWidget::name(), array()); ?>
qiangxue commented 11 years ago

I think we have enough discussion on this matter. I just checked in the change. Thanks to all for discussion.

creocoder commented 11 years ago

@qiangxue Than last question. If you say that $instance::staticMethod() is incorrect. How to correct code like this? This from behavior:

$owner = $this->owner;
$query = $owner::find();
$db = $owner::getDb();

This works fine, but if this is incorrect, can you show correct version of this code?

qiangxue commented 11 years ago

I don't know why it works (it's your code, right?)

I think the proper way is (note that static methods can be also used in a non-static way, but not vice versa):

$query = $this->owner->find();
$db = $this->owner->getDb();
creocoder commented 11 years ago

@qiangxue This works because $instance::staticMethod() works fine in PHP 5.3, but if you say this incorrect i'll use $instance->staticMethod() variant. Thanks.