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

Namespacing Classes #49

Open
HendrikN opened this issue Aug 6, 2012 · 7 comments
Open

Namespacing Classes #49

HendrikN opened this issue Aug 6, 2012 · 7 comments

Comments

@HendrikN
Copy link

HendrikN commented Aug 6, 2012

I'd like to integrate the GeoPHP project into our companies CMS, there is however a problem with the classnames. For example in some client-installations there's already a Point object defined. This would be solved by namespacing the Classes (e.g. geoPHP_Geometry_Point).

I absolutely realise that adding namespacing breaks backwards compatibility, but I do like to know what your opinion is regarding this subject.

If this is not the place to start this discussion, feel free to disregard and/or remove this issue!

@phayes
Copy link
Owner

phayes commented Aug 6, 2012

Hi @HendrikN,

I definitely want to support name-spacing, but yes am I am also concerned by backwards compatibility. I would welcome a pull request that adds name-spacing

@faegi
Copy link

faegi commented Aug 7, 2012

If namespacing then please use "proper" PHP (5.3+) namespacing and not just classname prefixes

e.g.

namespace GeoPHP\Geometry;

class Point extends \GeoPHP\Geometry {
...
}

Yes, it really breaks backward-compatibility and also only works with PHP 5.3+, but quite frankly anyone still on PHP 5.2 really should be upgrading anyway.

I have a local branch in production use which is namespaced like this (amongst other additions): I will see whether I can get permission to release it.

@HendrikN
Copy link
Author

HendrikN commented Aug 7, 2012

I'm not sure about using 5.3 namespacing... For example, we're planning on upgrading our envorinmont to 5.3 somewhere this fall and a lot of other hosting-companies will be on 5.2 for some time. (Some of them just upgraded to 5.2).

Although I have no clue what the "trend" is among other PHP libraries, I do think however that only supporting 5.3+ will exclude a lot of users who just can't upgrade that easily due to corporate restrictions.

@Jelle-S
Copy link

Jelle-S commented Apr 19, 2016

Last comment was 2012. I think it's safe to say that namespacing the classes wouldn't hurt anymore (even on shared hosting).

@BathoryPeter
Copy link

A pull request with namespaced classes is just waiting to merge: #125

@gholol
Copy link

gholol commented May 15, 2016

@BathoryPeter I looked at your implementation and it's wrong.

use geoPHP\geoPHP;
use geoPHP\Geometry\Geometry;
use geoPHP\Geometry\GeometryCollection;

You should have a fully qualified class name that should have a top-level namespace name, aka "vendor namespace". You also MUST use CamelCase.

So the proper way to implement that would look like this:

use Phayser\GeoPHP\GeoPHP;
use Phayser\GeoPHP\Geometry\Geometry;
use Phayser\GeoPHP\Geometry\GeometryCollection;

Implemented in #130

@BathoryPeter
Copy link

I don't think it is wrong rather an open question. Patrick has to decide about the naming and he / anybody (or me) can refactor it in less then a minute. I'ts just two click in PhpStorm.

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

5 participants