Reply to thread

Message

<blockquote data-quote="habtard" data-source="post: 456403" data-attributes="member: 90778">I mean by the standards of this community, its okay. There's a couple of things I want to mention which have probably already been said so I'll keep it brief.Everything is static, which means it has a global mutable state. Being able to call the static method which holds state is the worst. If you want to know why globals are so bad, create a small method which calls ::getInstance on some class and load some global, then challenge yourself to write a unit test.Echo and exit, its probably better if you throw some kind of exception here. It's rarely a good idea to kill the script like that.You wrote a class which judging from your documentation is a session handler. You don't have any custom logic in this class so it seems like you're over complicating it. Lots of little useless blocks of code here, like checking if a session has been started before destroying it, which PHP probably already does under the hood. Especially when you're calling session_start early in your script, before even initiating the handler class.Start following PSR-7, your coding conventions are all over the place.You're using echo to print views or &quot;responses&quot; to route callbacks, might be a better option to return actual HTTP responses.You switched to SimpleRoute for a more powerful router, but you're using less than half of its features. I would spend time writing your own router, with only the features you're actually using if you cared that much.I would also suggest you avoid using require_once, its generally only good to avoid something accidentally being redeclared in multiple libraries.Constant use of __DIR__, create a define(&quot;DIR&quot;, __DIR__); and start using a constant.There's lots more but I won't go on.</blockquote>

[QUOTE="habtard, post: 456403, member: 90778"] I mean by the standards of this community, its okay. There's a couple of things I want to mention which have probably already been said so I'll keep it brief. Everything is static, which means it has a global mutable state. Being able to call the static method which holds state is the worst. If you want to know why globals are so bad, create a small method which calls ::getInstance on some class and load some global, then challenge yourself to write a unit test. Echo and exit, its probably better if you throw some kind of exception here. It's rarely a good idea to kill the script like that. You wrote a class which judging from your documentation is a session handler. You don't have any custom logic in this class so it seems like you're over complicating it. Lots of little useless blocks of code here, like checking if a session has been started before destroying it, which PHP probably already does under the hood. Especially when you're calling session_start early in your script, before even initiating the handler class. Start following PSR-7, your coding conventions are all over the place. You're using echo to print views or "responses" to route callbacks, might be a better option to return actual HTTP responses. You switched to SimpleRoute for a more powerful router, but you're using less than half of its features. I would spend time writing your own router, with only the features you're actually using if you cared that much. I would also suggest you avoid using require_once, its generally only good to avoid something accidentally being redeclared in multiple libraries. Constant use of __DIR__, create a define("DIR", __DIR__); and start using a constant. There's lots more but I won't go on. [/QUOTE]

Verification

Reply to thread

Connect with us

Newest members