class_exists($class) fails with Yii 1.x autoloader

[bkonetzny]

We combined our Yii 1.x project with composer. Due to the nature of Yiis autoloader, checking class_exists() in Yasumi::create() will fail with an exception, as Yii doesn't know the class "Spain" (Spain.php) for example. For us, it seems the problem would be fixed by changing class_exists($class) to @class_exists($class).

As far as I understand, this change wouldn't cause any issues in the logic for loading custom classes for other use-cases, right? Please tell me if you would approve such a change, I'd be happy to provide a PR and tests for this.

[stelgenhof]

My apologies for the late reply. Would you mind sharing an code example how you use Yasumi with the Yii framework? It helps me better understand and can see if I can replicate the issue.

However, using the error control operator (@) is not really a fix. It only suppresses any error message that might occur. It would be better to handle the exception in your code.

[bkonetzny]

Reproducing this is rather complex, as Yii 1.x doesn't work with composer easily out-of-the-box. As far as I can see, Yii throws an exception if a requested class cant be loaded. Yii can find "\Yasumi\Provider\Spain" but not "Spain", so using a fully qualified class name would work in theory.

Examples:

Yasumi::create('Spain'); // Yii throws exception, class "Spain" not found
Yasumi::create('\Yasumi\Provider\Spain'); // Success

We are trying to load holidays based on language and use the provided helper method you created. This resolves the ISO code to the class name - which is not fully qualified, resulting in the above described problem

Yasumi::createByISO3166_2('ES'); // Yii throws exception, class "Spain" not found

Prepending the error control operator to class_exists fixed the error for us, but I see your point. We cant fix this from the outside, so I'd suggest an general fix for "broken" classloaders like ours. So if you want, I can rewrite the PR to use a try/catch on the class_exists in Yasumi::create?

[MatthiasKuehneEllerhold]

These are my 2 cents on this: The problems lies in the Yii 1.x autoloader, not in yasumi. Autoloaders must not throw exception if they can't find the class: they are expected to return false. So (as you said) this autoloader seems to be broken. Therefore I think IMHO that it should be fixed in the broken piece of code and not in a (framework-agnostic) library thats depended on a correct implementation. Is Yii 1.x still supported (1.1.18 came out in april 2017)? So maybe thats a route that can be taken.

[stelgenhof]

I think you would then looking at emulating Composer's autoloading feature. Alternatively you could create your own autoloader using the __autoload function (pre PHP 7.2) or the spl_autoload_register function.

As for your comment "We are trying to load holidays based on language", I am not sure that that is a good approach as some languages are spoken in more than one country (e.g. English, German, ...). For that particular reason, locales are created ('en_US', 'en_GB', etc.)...

[bkonetzny]

Both of you are correct! Thanks for the replies. I'm experimenting with setting Yii::$enableIncludePath = false; which seems to fix the issue of throwing exceptions when loading unknown classes. I'll get back to you once I have confirmed this works.

[stelgenhof]

@bkonetzny Could you tell if you were able to get it to work with Yii? If so, I'd like to close this issue as to my view nothing is wrong with the Yasumi library :)

[bkonetzny]

@stelgenhof Ah, sorry, yes - this seems to work. So I'd say this issue and PR can be closed. Sorry for the hassle.

[bkonetzny]

For reference: This is the Yii documentation about 3rd party loaders, setting $endableIncludePath to false like described there fixes the issue: http://www.yiiframework.com/doc/guide/1.1/en/extension.integration#using-3rd-party-autoloaders

[stelgenhof]

@bkonetzny Thanks for the update! Happy to hear it works now.