search

Deadpikle / yamf-php

YAMF (Yet Another MVC Framework) is a small PHP MVC framework designed to let you start using the MVC paradigm quickly without a lot of setup work
MIT License
2 stars 0 forks source link

Implementing PSR-4 #4

Closed Leprechaunz closed 6 years ago

Leprechaunz commented 6 years ago

Description of the pull request

Implemented PSR-4 autoloading #1

Description of tricky code

I tried to make changes related only to PSR-4, but actually touched couple of issues. If it's needed I can revert them.

All classed are now using namespaces and use to import other classes. All autoloading logic is done using composer. If new autoloading rules are needed we can add them to composer.json (not forget to regenerate autoloading dump with composer dump-autoload command.

Besides this:

  1. I added .idea folder to .gitignore
  2. Added type hinting to some methods. Like Request $request for controller methods.
  3. Fixed ErrorMessage: $msg was never assigned value
  4. I think it makes sense to follow PSR-2 as you already following PSR-4 https://www.php-fig.org/psr/psr-2/

All PHP files MUST end with a single blank line. The closing ?> tag MUST be omitted from files containing only PHP.

Deadpikle commented 6 years ago

Actually, in looking at the PSR-2 spec a little more closely, it looks like we're not perfectly following it yet with this PR -- mostly with the brackets for functions/classes. That's OK -- go ahead and leave the changes you did make towards PSR-2 compliance in, and I can file a code improvement issue for full PSR-2 compliance when this PR is merged. 😄

Deadpikle commented 6 years ago

Oh. I just realized why you would suggest capitalizing the yamf/**models** folder. Then you don't have to add more to the composer.json autoload, and moreso for subfolders if the user adds them to the Controllers or Models folders. I misunderstood.

However, it looks like a use statement is not case sensitive. use Yamf\Models\ErrorMessage; works just as well as use Yamf\models\ErrorMessage;. So, in theory, couldn't we just capitalize Yamf/Models without actually changing the folder name? Am I doing something else wrong? (It's arguably a bit late and I may not be thinking clearly.)

I see that the way a Laravel app handles it is to keep all the main dir's folders lowercase, and then subfolders can be uppercase. I suppose I would not be opposed to doing it the same way...I was trying to avoid putting controllers/models in an app folder, though, just so this wasn't like a stripped-down Laravel or somethin'.

It's getting late here (10 PM) so I'll respond with changes you make tomorrow my-time.

Deadpikle commented 6 years ago

OK, yes, clearly I am not thinking clearly. 😓 PSR-4 states:

The subdirectory name MUST match the case of the sub-namespace names.

So, let's capitalize the yamf/Models directory. I'm sorry, that misunderstanding was my fault. Now for bed so that I CAN think clearly tomorrow 😅.

Deadpikle commented 6 years ago

Thanks @Leprechaunz! I'll work on updating README.md and a few other things. :)