Skip to content
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

Put all the classes in their correct namespace #41

Closed
alombarte opened this issue Jan 13, 2014 · 8 comments
Closed

Put all the classes in their correct namespace #41

alombarte opened this issue Jan 13, 2014 · 8 comments
Assignees
Milestone

Comments

@alombarte
Copy link
Contributor

No description provided.

@ghost ghost assigned lombartec Jan 22, 2014
@lombartec
Copy link
Member

Hello @alombarte , I took some of the work @alexgt9 has done in https://github.com/alexgt9/sifo-composer , merged it with mine and pushed it to my fork here https://github.com/lombartec/SIFO/tree/sifo3-beta there you can see a first approximation of this task, please, comment about anything that can be improved 😄

@alombarte
Copy link
Contributor Author

The usage of namespaces affects mainly in those classes that have a subfolder like Cache or API. I've seen you renamed classes like CacheBase to Base and I'am wondering if it wouldn't be better to just rename the file to CacheBase.php and have the namespace Sifo\Cache\CacheBase (Base is a shitty name anyway, but we will take care of this when we ended this huge milestone).

Personally I think there is no harm in redundancy in the name "Cache" (having Cache/CacheBase.php)and helps later on identify the files. Whatever we do we should reach an agreement and apply it to all the classes.

The classes inside Filter.php should be separated and create the folder Filter to put them all inside. Same with Exceptions.

What do you think?

@lombartec
Copy link
Member

I completely agree, I already have done the filter and exceptions thing ib
my local repo, just wanted to get more information with this first
approximation. About the cache base and so on I will leave their names to
the previous. Thanks! 😃

@alexgt9
Copy link
Member

alexgt9 commented Jan 23, 2014

I'm agree with Albert, Base is shitty name. Let's put just correct namespaces in the actual classes.

@nilportugues
Copy link
Contributor

In order to avoid modifying every single controller and php file in the core, I'd use the following approach

<?php

namespace <Vendor>\<Whatever>;
use <Vendor>\<Whatever>\<RealClassName> as <OldClassName>

Specially in the largest files, such as Bootstrap.php

@alombarte
Copy link
Contributor Author

I am not scared on modifying class names inside Sifo library where we have the full control. If we change the real class name at some point that has a direct effect on people already using Sifo who could be directly using those classes. That has to be taken into account.

Using the use X as B approach only saves you from a little bit of work but adds noise and confusion for the future. In phpstorm renaming a class is as easy as pressing Shift+F6 and type the new name, so if we change a class name, let's spread it everywhere.

@nilportugues
Copy link
Contributor

Sure, this approach is just a fast way to work until all the classes get their proper namespace :)

@lombartec
Copy link
Member

I consider this task closed with this last PR #54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants