-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SYSDIR #25
Comments
This constant is useless, I'm voting to remove it. |
I agree with @narfbg. In the 10 or so years that I've been working with CI, I don't think I've ever found a need for that. And it's handled by the $systemFolder variable if you want to change it. |
It's been removed. |
I think that the same remove can be made and on other constants. Path Class is a lot better choice to register and handle paths.. |
I don't see a path class being beneficial here. The way CI has always done it is simple, by creating constants for paths that are constant. They're available sitewide. And they use less resources than a class would. |
I can't give any 100% important reason to have class instead of just constants. Instead all over the code to check if file_exists(SOMEPATH. 'folder/file') and to require it.. Of course this has something to do and with the Loaders.. |
Like anything else, OOP design can be taken to extremes it really doesn't need to be. In this case, you're right, that's more to do with a loader, or some other class. But if you go the route of creating OOP wrappers for all of PHP's non-OOP functions, things get a little crazy. :) In this case, your example, while a valid function (not necessarily requiring an entire class, but could be ...) would replace I'm not saying your class idea doesn't have certain appeals for different situations, but for this one, I think it's overkill and complicates things instead of simplifies them. |
windows OS
define('SYSDIR', trim(strrchr(trim(BASEPATH, '/'), '/'), '/'));
var_dump(SYSDIR);
string(0) ""
Fix:
define('SYSDIR', trim(strrchr(trim(str_replace('', '/', BASEPATH), '/'), '/'), '/'));
The text was updated successfully, but these errors were encountered: