More consistency in capitalization

[vlakoff]

Hi, I'm always confused with capitalization in CI, so I decided to gather the current behavior:

Controllers

• filename has to be lowercase

Models

• filename has to be lowercase
• $CI->load->model('themodel') can be lower or uppercase, but:
  • $CI->themodel has to use the same case
  • if $CI->load->model('themodel') then $CI->load->model('Themodel'), the class is loaded in CI twice

Libraries

• filename can be lower or uppercase (edit: has to be capitalized as of 3608e1a)
• $CI->load->library('thelibrary') can be lower or uppercase
• $CI->thelibrary can only be lowercase

Helpers

• filename has to be lowercase
• $CI->load->helper('thehelper') can be lower or uppercase

$routing['controller'] override in index.php

• has to be the same case as the controller filename, so lowercase, and without the .php extension

As you can see, there are some discrepancies.

What are you suggestions to improve consistency and/or flexibility, without breaking existing apps?

[pickupman]

This can prove to a problem for those that develop on a Windows environment, and then deploy in a Linux environment. From your notes, lowercase appears in each of the scenarios, so I would say for personal preference stick with lowercase whenever possible. Less issues between environments. The exception to the rule is when you are overriding a system library/helper. You file and class need to match the original case.

[narfbg]

As of 3608e1a, library file names can no longer be in all-lowercase.

On another note - property names are case-insensitive and this doesn't depend on no condition, they just are. Therefore it doesn't matter if it's $this->library, $this->Library, $this->whatEver, etc.

[timw4mail]

To clarify, most of this case-insensitivity is because of PHP, not CodeIgniter.

[vlakoff]

So we have an all-lowercase filename rule, except for libraries which have to be capitalized. Not too hard yet, but not optimal. However I guess that can't be improved.

The points that might be improved IMHO are:

• What I described about load->model(). Having load->model('foo') and load->model('Foo') at different locations of an app doesn't sound inconceivable to me.
• $routing['controller'] override in index.php might be more resilient (see 4876b48 for an example of an issue I ran into)

[narfbg]

The general rule that we're going towards is classes in Ucfirst and anything else in lower-case. This could cause backwards compatibility issues for controllers and models however and is still discussed with EllisLab.

Note that I'm talking about filenames only here as this is the only thing that cannot be solved by PHP itself if you didn't follow the rule. With that said, the points you've listed should be considered bugs if it is indeed a problem - when you have a rule of how should the files be named, then we can easily strtolower() and/or ucfirst() the input string.

[AkenRoberts]

Wasn't the point of checking for multiple variations of the filename to support the differences between Windows and Linux? I'm all for providing more consistent conventions recommended in the user guide (I've been matching my file names to the class name for a long time), but I thought that's why we also checked the all-lowercase filenames, too?

On another note - property names are case-insensitive and this doesn't depend on no condition, they just are. Therefore it doesn't matter if it's $this->library, $this->Library, $this->whatEver, etc.

They are case sensitive, just like variables. Functions and methods, however, are case insensitive (maybe that's what you're thinking of).

[narfbg]

Um, no ... I was thinking of class names and given that they are case-insensitive I'd assume properties are as well. I've never actually tried mismatching the case however, lol.

[AkenRoberts]

Haha gotcha.

[narfbg]

On another thought, CI_Controller::__get() already exists, so fixing the property issue shouldn't be a problem.

[ragboyjr]

what needs to be done to get this issue rolling?

[AkenRoberts]

Well, what do you suggest?

[ragboyjr]

Well, it seems like everything is going to psr-0 naming conventions, I think it would be easiest if we followed something similar with models, and libraries. It seems to make things much simpler when the class name you load is the same name as the file. And you could always add backwards compatible (deprecated) feature that will strtolower the file name if the name given doesn't exist. i.e. user calls $this->load->library('Lib_NAME'), so we'd check ./application/libraries/Lib_NAME.php, if doesn't exist then we load ./application/libraries/lib_name.php. That's kind of how a psr0 loader works which makes sense to me, and seeming how every other php framework out there follows this model, it must make sense to others too.
I think it'll also allow more flexibility for codeigniter in the future if wanted to support namespaces, autoloading, composer, etc... (not to say I want to propose any of those changes).

As far as helpers go, they really are just a prettier .inc file, and it's the convention to make inc files lowercase, so I think we should keep helpers the same (all lower case) as far as capitalization goes.

Examples

Here are just some examples of what users would code, and what the actions created would be.

The following code is not independent, and I figure $this->load->model and $this->load->library should work exactly the same as far as including files, capitalization, and variable names are concerned.

// essentially calls require './application/models/Model.php'
$this->load->model('Model');
$this->model->func();

/*
 * tries to call require './application/models/MODel.php'
 * if doesn't exist then calls './application/models/model.php
 * it will overwrite the $this->model, unless a second parameter of var name
 * was specified. 
 */
$this->load->model('MODel');
$this->model->func();

// tries to load require './application/models/moDEL.php'
$this->load->model('moDEL', 'moDEL');
$this->moDEL->func();

/*
 * tries to call require './application/models/Model/Stuff.php'
 * if that fails, then it tries './application/models/model/stuff.php'
*/
$this->load->model('Model/Stuff.php');
$this->stuff->func();

/*
 * tries to call require './application/models/Model/Foo.php'
 * if that fails it tries './application/models/model/foo.php'
 * The instance name will be strtolower(str_replace('\\', '_')),
 * and the class name to instantiate will be
 * $model = new Model\Foo();
 */
$this->load->model('Model\Foo');
$this->model_foo->func();

[vlakoff]

Sounds very good.

I don't understand why library filenames, and only them, would have to be capitalized, this just doesn't make sense.

Meanwhile, the Model loading discrepancies I mentioned in my OP would happily be fixed ;-)

[narfbg]

@ragboyjr Don't bother with PSR-0 for the time being. We are indeed most likely going that way, but due to support for PHP 5.2.4, it's technically impossible at this point.

@vlakoff I believe I already mentioned that there are plans for everything to go Ucfirst.

Glad you brought this issue again though, it's a release-blocker. It was easy to switch libraries, as custom libraries are not vital for a CI application and lower-case names for them are not that common. Controllers and models are however and we need to work out a smoother transition for them. I'm thinking of still allowing them to be all-lowercase, but deprecate them and log an error message when a non-Ucfirst file is loaded. Anybody got a better idea?

[vlakoff]

@narfbg Can you please edit your first post on this topic? You wrote:

On another note - property names are case-insensitive and this doesn't depend on no condition, they just are. Therefore it doesn't matter if it's $this->library, $this->Library, $this->whatEver, etc.

That's wrong.

I've just double-checked, it doesn't work. There is no stuff like __get(). A capitalization error just throws a PHP notice (and of course the property isn't obtained), maybe that's why you missed it.

[narfbg]

You should've just read the rest of the comments. :) This isn't a wiki page ... if I edit something, I should also delete another few posts.

[ragboyjr]

I'm down with @narfbg's solution, we'd just need to stress this fact in the documentation. Btw, for ci 3.0 does the loader support namespaced loading?

[AkenRoberts]

What do you mean? Namespaces are not natively handled at all since the minimum PHP requirement is still 5.2.x.

[ragboyjr]

Doesn't codeigniter support 5.2+? So it wouldn't be able to support php code that is 5.3+ which would be a deterrent for people using CI 3.0 with php 5.3+. A lot of new code is written using namespaces, so people (myself included) wouldn't be able to port namespaced code into Codeigniter without the hassle of renaming all of the files

[AkenRoberts]

It's not that it can't handle namespacing, it's that it isn't meant to. You can use namespaces and whatever 5.3 or 5.4+ code you want in CodeIgniter, you just need to set it up properly. Typically that means adding an autoloader that matches whatever file hierarchy you have. If you use Composer, all you need to do is add one require statement to your index.php (or similar), and you can use those libraries like you would anywhere else.

You shouldn't want or need to "port" anything backwards into CI (CI is behind the times, you don't want to dumb your code down if you can help it). You should just use third party code like you would anywhere else, with the new \Namespace\Class syntax or similar.

[ragboyjr]

K, so you're saying, assume people are using an autoloader for newer code (code with namespaces) (which is a fairly good assumption, I might say), and CI's loader won't support namespaced code because an autoloader would do a better job. Agreed?

[AkenRoberts]

I still don't really get what you're asking. CI's loader is much more involved than a simple autoloader that require's a file automatically. Comparing it to a Composer-style autoloader is not the same. And the whole discussion is moot for the moment anyway since CI isn't designed to handle namespaces at all anyway.

[ragboyjr]

Agreed, whether or not CI's loader supports namespaces is irrelevant to this thread. I thought it was a simple question. I'm still down with @narfbg's solution

[ragboyjr]

Let's try to get a working draft of what needs to be done to implement this issue. I'm not saying this is necessarily how it needs to be implemented, but we have to start somewhere.

1. Allow the loader to load files in different types of naming. We will try to load the files in the following order
   • Load file using the same string passed in (i.e. $this->load->model('MOdEL'); // trys to load 'application/models/MOdEL.php'
   • Load file using ucfirst(strtolower($str_passed_in_from_user))
   • Load file using all lowercase
2. Here's where it gets tricky, how exactly do we decide on how we instantiate the class? Do we do the same thing as above (i.e. instantiate the string given, if doesn't exist, instantiate ucfirst, else all lower)?
3. Add a log message when we have to lowercase the string given to load/instantiate the class given.

The problem I have with allowing these three different options is that there is a runtime performance hit. We have to check if a file exists and if a class exists before we can actually require a file or instantiate the class. If we force CI users to use a specific naming convention when using the loader, then we can just require the file needed and instantiate the object without having to do that error checking first.

The problem with that solution is that it doesn't give users flexibility to name their files how they like to. So I'd propose, we'd force the naming convention of we require the file with the exact same name passed and instantiate the class with the exact same name passed in. So that way we still give users the ability to chooser their naming conventions, but we don't have to go through the runtime performance ding of error checking. The only thing we'd enforce users is that their class names and file names need to be the same, but that's kind of the standard anyway in php.

I know the performance ding isn't that much, but little things add up. And what we can do for error checking is put the require and instantiation code in a try/catch block so if there was an error we can properly report it, and try/catch blocks don't hurt runtime performance unless an error occurs.

This should lay a nice framework for what needs to be done, so everybody else can modify these points until a final decision is made.

[marcvdm]

I think @ragboyjr nails it with his last comment.

So I'd propose, we'd force the naming convention of we require the file with the exact same name passed and instantiate the class with the exact same name passed in. So that way we still give users the ability to chooser their naming conventions, but we don't have to go through the runtime performance ding of error checking. The only thing we'd enforce users is that their class names and file names need to be the same, but that's kind of the standard anyway in php.

[narfbg]

2029231

Classes -> Ucfirst
Everything else -> lowercase

[ivantcholakov]

@narfbg
About 2029231 :

I think, that backward compatibility with the old filenames for classes should be preserved. It would not be very diffcult.

Also, the third party "Modular Extensions" feature ( https://bitbucket.org/wiredesignz/codeigniter-modular-extensions-hmvc ) should be kept running, because it is very popular solution for HMVC at the moment. I mean the method _set_routing(), it should stay with public access.

[narfbg]

IMO, maintaining BC makes the whole change pointless + this is not something that I've decided on my own.

Third party extension developers should find their own ways into updating their code, not the other way around.

[ivantcholakov]

I realize, it is not your own decision. So, I am just leaving here some notes "for the record".

Within a transitional period both of the controllers' class/file naming conventions (the old and the new) may be implemented. It is not that hard. The old convention may be deprecated in CI 3.0 and it may be removed in CI 3.1. This is the correct way maintainers of third-party packages/sparks/cms system, etc. to get the signal about this (radical) change.

CI 3.0 had a long run, on the other hand CI 2.x is too old. This is why I am using the developer's version on real sites. OK, I'll manage this situation, but many others by adopting the same decision would suffer.

BC and tons of third party stuff are the decisive advantages of CI, it is not good such a huge legacy to be scrapped just like that. I think this radical change without transitional period would be a big disappointment to the fans. Please, advice your bosses. :-) Let them think about this again.

[narfbg]

Since we're saying stuff for the record - I didn't say that I didn't have a say in this, I'm not blindly defending a decision somebody else has made. What I meant was that this has been thought about by more than 1 person.

And with that said, we all care about BC, we know what a transitional period is, why and how it's done. And nobody said it's hard to do - it is indeed easy.
However, when it comes down to the filesystem and not just some code, then it's something that just has to happen. A transitional period won't save you from doing it eventually and many future changes depend on this one in particular, so adoption rate is important here.

I'm not discussing third party code - again, the respective authors of such code should address that, we only deal with our own code here.

Also, I don't have bosses. :)

[vlakoff]

A major version is the best moment to break BC. Doing it in 3.1 would be a very bad idea.