Yii3 Properties


(Alexander Makarov) #30

You forgot property annotations you need to have in order to get IDE autocomplete.


(Mike) #31

Can’t we add them in the class comment header as usual?

/**
 * ....
 *
 * @property string $foo some foo for bar
 */

(Alexander Makarov) #32

We can. I’m pointing to “less” points.

In case of plain class that’s:

class Post
{
    private $title;

    public function __construct(string $title = '<please write some text>')
    {
        $this->validateTitle($title);
        $this->title = $title;
    }

    public function getTitle(): string
    {
        return $this->title;
    }

    public function setTitle(string $title): void
    {
        $this->validateTitle($title);
        $this->title = $title;
    }

    private function validateTitle(string $value)
    {
        if (mb_strlen($value) > 255) {
            throw new \Exception('invalid!');
        }
    }
}

In case of your variant it’s:

/**
 * @property string $title
 */
class Post
{
     use PropertyTrait;

     protected $_title;

     private function properties(): array
     {
         return [
             'title' => [
                 'validate' => function ($value) {
                     if (mb_strlen($value) > 255) {
                          throw new \Exception('invalid!');
                     }
                 },
                 'default' => function() {
                     return '<please write some text>';
                 }
             ]
         ];
     }
}

First variant is a bit more verbose but is way less magical.


#33

Not really. With getter and setter you also have 2 methods which could be part of interface for example. And syntax is clear and obvious, while props() approach is another magical solution which needs to be learned.

In general I would recommend to NOT use magic attributes whenever possible. They may look like a great idea at first look, but the bigger the project, the more WTFs you will get. Using getters and setters directly usually gives less confusion (for both - programmers and SCA tools) and saving 3-5 chars is not worth the trouble.


(Mike) #34

Well I agree to all you said. But on the other hand this means that Yii 3 will be quite different from all its predecessors. In my view one of Yii 1 and 2’s goals was to let you write code that keeps boilerplate at a minimum. Remember one of its marketing claims was “Less noise - more signal”. I liked that a lot.

Looking at Yii 3 I understand the motivations and goals (SOLID, GRASP, etc.) and also think that’s the way to proceed. But this also comes at a cost: It often means that code get more “java-ish” which equals to more noise in your code. I’m curious if this can be solved so that we can keep the Yii feeling we all like.

On a sidenote: If magic methods are avoided (which I can understand), how will this affect ActiveRecord? With PRADO (Yii’s predecessor) we had to define all DB columns as public properties of your class. IMO this would be an acceptable tradeoff. Or will there be any AR implementation in Yii 3 at all?


(demonking) #35

Because of AR, at the moment no implementation are planned.

Discussion : Using Cycle ORM

Reason : Using Cycle ORM


#36

I wouldn’t say that using getter explicit is a noise. It is actually signal - it shows you what is really happening. For example it is quite strange that you can’t use $this->someArray['someKey'] = $someValue;. But it becomes more obvious if you write it as $this->getSomeArray()['someKey'] = $someValue;.

In general I don’t think that magic attributes are that important to keep “Yii feeling”. I would be more concerned about removing some known patterns (Yii::$app - it may be ugly, but is is really convenient and easier to understand and control in small projects) or parts of original framework (URL manager, AR, console).


(Lazarevic Ivica) #37

I hate writing Java classes with bunch of trivial getter and setters. Also I hate Java for spending more time configuring than programming. I hope Yii3 is not following Java path.


(Alexander Makarov) #38

I am as well :slight_smile: Will try to avoid unnecessary boilerplate where possible but not at the cost of clarity of what actually happens.

Active Record is an exception there. Can not be done without some magic in case we don’t want to define properties manually.

Maybe.

Yii::$app is a bad and convenient thing at the same time. It is extremely easy to misuse resulting in testing and maintenance hell. In your apps you’ll be able to do it easily if needed though by defining a global class holding container right in index.php.

Current URL manager is different but porting old good one is possible. It is not a priority for me personally though since interface is in place and it works by leveraging FastRoute.

As for AR and console, sadly, I don’t have enough time to port these myself. But if there will be someone willing doing that, I’ll help with reviews, directions and will gladly accept it as part of the framework.


#39

That is understandable, but you should really ask yourself what is the point of this framework. For me Yii was some kind of balance between Laravel (which allows you to write unmaintainable code really fast) and Symfony (which is SOLID and promotes “right way” of doing things, but at cost of simplicity). Yii 2 is simple and easy to understand, without unnecessary abstractions and ceremonies, but it still scales quite well and you can build big applications without pulling hair out of your head.

In Yii 3 my two favorite parts of the framework (routing and DB layer) are gone, and framework lost much of its simplicity. It is much more like Symfony now. And it is not necessary a bad thing, but if I would need a Symfony-like framework, I would choose Symfony instead of Yii 3. I’m not feeling like Yii 3 have some killer-feature that could change that…


(Alexander Makarov) #40

Yii 3 moved slightly away from magic and fragility that were present in Yii 2: properties (magic), behaviors (magic), lack of encapsulation (fragility), ability to affect any application component from anywhere while the components were not designed for such use (fragility). It was not bad at all in skilled hands of developer who read the guide, checked source code and memorized conventions but often was disastrous in hands of less skilled developer.

Yii 3 is now uses common overall PHP practices almost without magic and fragility present. Yes, it’s more verbose compared to Yii 2. It contains less complexity than either Yii 2 (because less magic) or Symfony (because no complicated compilation, caching or config present). There are no unnecessary abstractions (if there are any, please report these as bugs). It should scale much better than Yii 2 did because testing is easier and because it prevents practices that certainly lead to less maintainable code.


#41

There was always magic cache in Yii 2 (DB schema cache, especially annoying in AR context), and Yii 3 introduces composer-config-plugin for automagic configuration.

I already raised my concerns about cache component, without any effect. There is Yiisoft\Cache\CacheInterface, but it is not implemented by cache backends (like ArrayCache). So now we have Yii cache components, which require a separate layer of abstraction (Yiisoft\Cache\Cache decorator) so they could behave as Yii cache component. ArrayCache could implement Yiisoft\Cache\CacheInterface directly, so there would be no need for decorator - there is nothing wrong with having PSR implementation with extra functionality.

Cache component was the only one I reviewed, but it really made me worry how pragmatic Yii 3 will be. Refactoring of this component made it a lot more complicated, harder to use and increased code duplication.


(Alexander Makarov) #42

Config merging was there in Yii 2 advanced app template. It was moved to composer plugin. And yes, this thing is kind of magical but alternative is manual config merging that I don’t like. As for caching, DB schema cache was not that magical compared to what is there in Symfony.

Thank you for that. I was thinking a lot about it and sorry if I hadn’t communicated clearly why it went the way it went. In case of not having Cache decorator:

  1. We’ll have to copy-paste what’s in the decorator for every cache backend then: array, memcached, wincache, apcu etc.
  2. In case of need to use third party PSR cache backend, user would be required to implement adapter first. Now it could be used right away.
  3. Current structure potentially increases non-Yii usage of cache packages which is beneficial to potential contributions.

Another thing with cache is that it’s a library that isn’t strictly tied to Yii itself. It could be used separately. As for refactoring, I guess you are referring more to splitting individual drivers into their own packages. There are benefits in doing that:

  1. Ability to release separately from other code.
  2. Ability to specify platform dependencies in composer.json.
  3. Potentially more usage by PHP community as a whole thus more contribution.

So yes, indeed in case of cache it become a bit more complicated in initial configuration but

$cache = new Cache(new ApcuCache());

instead of

$cache = new ApcuCache();

is a very low price to pay for benefits gained.

Also such trade-offs are mainly introduced where PSR interoperability is considered or libraries are meant to be used out of context of Yii as a framework. In yii- packages that’s not the case (or at least I’m trying hard to keep these extra layers under control).

Reviewing more from Yii 3 would be certainly helpful.


#43

I’m not saying that it is wrong, IMO it is unavoidable at some scale, but this is kind of Symfony bootstrap magic you’re complaining about.

Maybe, I’m not that experienced with Symfony to judge. But situation, when state of schema cache affects list of attributes available in AR model (so cache affects list of properties available in PHP class) is pretty close to worst possible magic.

You could use base class with shared code and implement only necessary methods - it worked in that way in Yii 2 and it was a lot simpler, not mention less code duplication.
Also, current design seems to create some artificial limitations resulting in the worse implementation of some features. For example, APCu has native add support, but ApcuCache is no using it, because add() is implemented at decorator level. So now ACPu backend uses less efficient and non-atomic implementation, ignoring native support of this feature in APCu. There would be no such problem if ApcuCache would implement Yiisoft\Cache\CacheInterface directly.

That is a good reason why decorator is implemented, but not explain why it is required by Yii backends. Yii backends could implement interface directly, and decorator would be required only for 3rd-party implementations.

I’m not sure how few additional methods in PSR implementation could change that.

Actually, this is the only part of this refactoring I like. :wink:


(Alexander Makarov) #44

Yes. Still, AR is exactly that if you need any acceptable level of performance. I guess that’s one of the reasons explicit mapping got popular despite being more cumbersome to handle.

Correct. See ↓

It’s not required by cache backends if you meant that. cache and cache-apcu (or any other cache package) aren’t inter-dependent now. Implementing decorator directly would:

  1. Cause specific cache packages to depend on main cache.
  2. Code duplication since we still need separate decorator.

If these issues are solved (or considered not important), I’m fine with having cache backends implement additional methods such as add() in a storage-specific manner.

You are correct. That is not a problem for our case.


#45

I don’t think that performance is the problem. Defining list of attributes and its types at PHP class level could save some cache-related WTFs and improve performance (no need for extra queries to fetch schema).

It is required if you want to target Yiisoft\Cache\CacheInterface in your app. If you want only Psr\SimpleCache\CacheInterface, then you have plenty of existing packages, Yii implementation does not give any additional value.

I don’t think this is a problem. yiisoft/cache does not have any additional dependencies, there is no technical problem with requiring yiisoft/cache by yiisoft/cache-apcu.

This is true, but:

  1. It could be limited by using base class for all classes implementing Yiisoft\Cache\CacheInterface, so only a few proxy methods would need to be declared.
  2. There is already a lot of code duplication (key or ttl normalization, redundant exceptions). I believe that using Yii 2 architecture (base cache class extended by specific backends) will reduce code duplication compared to current state of Yii 3 cache packages. Just compare APCu implementations (one of the simplest backends - does not even need serialization): Yii 2 vs Yii 3 - which one is simpler?

(Alexander Makarov) #46

This way you’ll loose main benefit of AR: dynamic schema introspection. You’ll essentially move it very close to data mapper.

I’ll take some time to see if cache can be revised: https://github.com/yiisoft/cache/issues/35


#47

What do you mean by that? The fact that list of available attributes in AR depends on state of DB is not a feature for me - it is a problem. AR is usually a base of model layer in the app, it is not good if you can’t relay on it as a type and its behavior depends on DB (and cache) state. In practice it is just another thing to worry about, because in most cases app will not work if DB state (or schema cache) is not in sync with PHP code (you may try to read value from non-existing column).


(alexkart) #48

There is more code in Yii3 version because several issues that were in Yii2 were fixed. The code is the same except those fixes.


#49

What issues? If there are bugs in Yii 2 implementation, they should be fixed there too. But I’m seeing mostly redundant keys and ttl normalization.