Welcome Guest, Not a member yet? Register   Sign In
user-library class loading
#1

[eluser]sipsniffa[/eluser]
Hi,

I recently ran into a problem when trying to load my own CodeIgniter library. I initially posted a query to the 'Code and Application' forum and finally posted on what may be a bug. The problem was related to the use of the 'subclass_prefix' configuration option and its possible misuse in the CI code base or missing documentation.

The problem is caused by trying to load a user-library (placed in application/libraries/) without assigning a value to the 'subclass_prefix' configuration option (i.e. 'subclass_prefix' took the default, empty string value). This results in user library classes being deemed attempts to subclass CodeIgniter library classes (code snipped from _ci_load_class() in system/library/Loader.php):
Code:
$subclass = APPPATH.'libraries/'.$subdir.config_item('subclass_prefix').$class.EXT;

// Is this a class extension request?            
if (file_exists($subclass))
{
    // ...
}
The decision of whether the user's library class ($class) is a subclass is in error. A user's library class is deemed to be a subclass by concatenating its name with the config value ‘subclass_prefix’ and the '.php' extension and then checking whether that file exists on the file system. When 'subclass_prefix' is empty, a user library class will always be deemed a CI subclass! Now when a CI base-class by the same name cannot be found the _ci_load_class() function bombs out with a show_error() call:
Code:
$subclass = APPPATH.'libraries/'.$subdir.config_item('subclass_prefix').$class.EXT;
    
// Is this a class extension request?            
if (file_exists($subclass))
{
    $baseclass = BASEPATH.'libraries/'.ucfirst($class).EXT;

    if ( ! file_exists($baseclass))
    {
        log_message('error', "Unable to load the requested class: ".$class);
        show_error("Unable to load the requested class: ".$class);
    }

    // ...

}

I think that the initial subclass test doesn't make sense when ‘subclass_prefix’ is the empty string. Might this be considered a bug, either in the documentation or in code?:

1. Documentation Bug
I haven't seen it mentioned in the docs, but if a client wishes to implement a CI user-library then it would be helpful if the docs stated that a value must be assigned to the 'subclass_prefix' configuration option - even when no subclassing of CI libraries is to undertaken.

2. Code Bug
One possibility that comes to mind as a code fix would be to prohibit library subclassing if the configuration setting 'subclass_prefix' contains it's default, empty string value ('').
I think this could be achieved by placing a guard around the subclass test code - which I highlighted above - like so:
Code:
// Subclassing only allowed when 'subclass_prefix' is defined.
if (strlen(config_item('subclass_prefix')) !== 0)
{
    $subclass = APPPATH.'libraries/'.$subdir.config_item('subclass_prefix').$class.EXT;

    // Is this a class extension request?            
    if (file_exists($subclass))
    {
        // ...
    }
}
I think this is a more elegant solution and demands less of the user than the documentation option.

While looking at _ci_load_class() there appears to be code that is ripe for refactoring.

I'd be grateful if someone could explain the process used by EllisLab when handling community offerings of code.

Thanks,

Paul.
#2

[eluser]xwero[/eluser]
I think it goes too far in helping newcomers to the framework. You can start idiot proofing every function and method but this results in a slower framework.

By default MY_ is added to the configuration setting. If developers don't use it they don't touch it. The scenario you paint here is a willful change in the configuration. If something goes wrong it should be the developer who has to look what causes the unexpected behavior and not let the framework pays for its wrongdoing.

Last week i had the same discussion in the feature requests section about reloading libraries in the autoload file that are loaded by default.
#3

[eluser]sipsniffa[/eluser]
[quote author="xwero" date="1229455365"]
I think it goes too far in helping newcomers to the framework. You can start idiot proofing every function and method but this results in a slower framework.
[/quote]
Somewhat emotive terms, but I get your point.

[quote author="xwero" date="1229455365"]
By default MY_ is added to the configuration setting. If developers don't use it they don't touch it. The scenario you paint here is a willful change in the configuration. If something goes wrong it should be the developer who has to look what causes the unexpected behavior and not let the framework pays for its wrongdoing.
[/quote]
I'm a newcomer to CI and wasn't aware that removal of the 'MY_' configuration value would cause problems for me later - maybe an over-zealous instict to have a clean config. Changes to configuration are on the whole wilful, I think.

[quote author="xwero" date="1229455365"]
Last week i had the same discussion in the feature requests section about reloading libraries in the autoload file that are loaded by default.
[/quote]
Very frustrating I guess.

Thanks,

Paul.
#4

[eluser]treehousetim[/eluser]
[quote author="sipsniffa" date="1229454223"]
I recently ran into a problem when trying to load my own CodeIgniter library. I initially posted a query to the 'Code and Application' forum and finally posted on what may be a bug. The problem was related to the use of the 'subclass_prefix' configuration option and its possible misuse in the CI code base or missing documentation.

The problem is caused by trying to load a user-library (placed in application/libraries/) without assigning a value to the 'subclass_prefix' configuration option (i.e. 'subclass_prefix' took the default, empty string value). This results in user library classes being deemed attempts to subclass CodeIgniter library classes ...[/quote]

I just solved this for my Page library ( application/Libraries/Page.php ) by putting this line at the top of the file

Code:
class CI_Page extends Page {}

It's not ideal but it keeps me from modifying the CI core which is helpful.

Sad




Theme © iAndrew 2016 - Forum software by © MyBB