Namespacing Classes #49

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

Projects

None yet

6 participants

@HendrikN
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
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
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
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
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

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

@gholol
gholol commented May 15, 2016 edited

@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

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