Repository navigation
APPPATH, SYSPATH and similar constants. Rename? #2
Description
Sorry for barking so early. :-)
Sample APPPATH, SYSPATH usage:
https://github.com/lonnieezell/CodeIgniter4/blob/develop/application/config/autoload.php#L33
Is it possible these constants to be renamed or name-spaced? Maybe in the distant future I'll make an attempt for compatibility layer about CI3. A name collision between certain CI3 and CI4 constants seems to me an obstacle for this goal.
Activity
lonnieezell commentedon Aug 28, 2015
Yeah, a bit early. See the readme :)
I've given some thought to a compatibility layer (unofficially) before, too, and I'm not convinced that this needs to be different. And it seems like it might help in some ways to keep them the same, because if the structure is kept with application and system folders, then anytime you use those paths in your application, they still point to the same general area. If you're setting up multiple applications that share the system folder, then your old application folder could use the same constants and still be correct.
I'm not 100% convinced this change is necessary, but it's definitely something to consider as we move forward.
jim-parry commentedon May 31, 2016
Further thoughts on this? If a change is to happen, then now (before first milestone) would be the time to do it.
For the record, I am not in favor of such a change -> the more familiar things we have, the easier I see it transitioning from CI3 to CI4 once ready.
lonnieezell commentedon May 31, 2016
I agree - not in favor of this one, in large part due to the familiarity aspect. While he does have a point for specific upgrade paths, I see upgrading from 3 to 4 as being a large enough of a project as to not necessarily be feasible. However, you could have both versions running side by side, and certain modules run on one version and another run on another version, the main tricky thing here would be handling routing, but should be possible.
jim-parry commentedon May 31, 2016
I am not convinced that we should provide for CI3 and CI4 side-by-side (in the same app, I presume) ... that feels like way too big a can of worms. I don't recall any attempt to do that with CI2 -> CI3, it was one or the other IIRC.
From what I am seeing, if the constants are extensively used in code (sounds like a good practice), then a conversion/migration guide, which referenced them, would be a better service than renaming the path constants.
lonnieezell commentedon May 31, 2016
OH, yeah, I wasn't saying that we go out of our way to allow to run side by side, but that's how any large migrations are likely to work - moving one part of the site at a time over. I've heard people on the forums talk about doing that between 2 and 3. The constants didn't provide any problems there, but the codebases were similar enough that it probably wouldn't have caused a problem.
ivantcholakov commentedon May 31, 2016
A suggestion without thinking too much:
FC_PATH
SYS_PATH
APP_PATH
... and the same rule for other that I might miss.
sv3tli0 commentedon May 31, 2016
I already suggested in other discussions that Path constants is OLD PHP practice and perhaps it must be replaced, either by path generator class or something like that. Specially with the new autoloader its even more comfortable to have some class to manage paths instead of some inline written paths using constants..
By using constants at each place where you use them you have to always check if the required file exists and then or else..
jim-parry commentedon May 31, 2016
Hmmm ... define CI4 constants for the path name (CI4_APP, CI4_FC, ...) and then have something like path(CI4_APP)?
Not sure ... this feels off.
Or CodeIgniter\Path class with app, fc, view, etc methods?
lonnieezell commentedon May 31, 2016
I see no problem with global constants for these type of things, personally. I know people like to have classes for everything and shy away from any "old school" PHP, but they all have their place. Besides, a class takes more CPU and memory to use than a constant. And having a new class just to hold static values is the type of thing I was always told was bad use of OOP. These are values that will never change throughout the request so a constant is the language feature intended to handle those. While we could add it to the Codeigniter class, and I'd be semi-ok with that, the use is even more complex for developers because it's no longer
It's getting a reference to the CodeIgniter class and then calling a method on that class to return a value that will never change. Seems overkill to me. Personally, the language features are still there and it's not bad form to use standalone methods, or global constants, etc, as long as they're used with care. Spaghetti code can be created in thoroughly OOP code (though I understand the term for that is lasagna code), so it's not that the feature is used, it's how it's used.
And we are trying to keep some of the flavor and feel of CI after all. :)
I also don't see a need to "namespace" the constants. It might make sense if we didn't have 10+ years of history with the old ones. As far as I can think of for the upgrade process, you're only ever going to be in one version or the other during a single request, so I can't see the constants interfering with each other. And, if they did, the system would crash, telling the dev what to fix.
While I'll go along with the majority on this, neither of these suggestions seem necessary or even like a great idea.
jim-parry commentedon May 31, 2016
I vote that we stick with the existing constants (their names). Migrating is going to be enough fun without changing them.
jim-parry commentedon Jun 5, 2016
Lets keep it as implemented.
Merge pull request #2 from puschie286/views_hints
Merge pull request #2 from bcit-ci/develop
Merge pull request #2 from bangbangda/filter-in-the-router
Merge pull request #2 from suddenly-eva/feature/websafe-routes-fix
Merge pull request #2 from codeigniter4/develop
Merge pull request #2 from thingNumber1/patch-2
when $_SESSION is null
Update CodeIgniter.php
Error if the table haven't primary key
Merge pull request #2 from codeigniter4/develop
fix: IncomingRequest::getJsonVar() may cause TypeError
fix: $fraction type
fix: ErrorException when the field is not sent
Update Connection.php
fix: Passing null to parameter error
fix: update toString() params