Namespacing Core Classes

[Andrew Short]

Hey All,

I'm currently creating a class as part of the nestedurls branch -
however due to it's possibly conflicting name I will be namespacing it
in order to avoid problems such as that mentioned in issue #2827.
Indeed, I think it is a good idea to namespace several core classes
which are likely to conflict, either due to code from the PHP core/
extensions or from other frameworks. Classes such as Session,
Controller and Int are a few I can think of off the top of my head
that, in my opinion, are likely to conflict.

However, the current practice of namespacing core classes with "SS"
when a conflict found should in my opinion be changed to proactively
namespacing commonly named classes with an "SS_" prefix, for a couple
of reasons:

* Since the ManifestBuilder no longer requires class and file name
matches, we can store a "SS_Controller" class in "Controller.php" -
allowing for a cleaner structure.
* The problem with "SS" becomes immediately apparent when you
namespace Session - "SSSession" is not the most pleasing name.
* I think we should proactively namespace common class names in order
to prevent issues recurring in the future, as well as easing
interoperability between sapphire and other scripts/components.

However, the main problem with changing namespace separator and adding
namespaces is backwards compatibility - we would have to rename
SSDatetime to SS_Datetime for consistency, and I would recommend
namespacing most of the DB field types due to their common names. If
this change were to be made then any core usage would have to be
replaced, as well as another upgrading requirements.

In order to address this, I have created a very basic prototype of a
refactoring script that can replace references to any namespaced
classes - and as such I would recommend that as part of 2.4 we
identify and namespace potential problems, as well as distribute a
separate CLI (or web based task) utility to rename namespaced classes,
in order to ease the transition for people upgrading to 2.4.

I'm sure that others will disagree with the various points I've made,
so I would really appreciate it if I could get any feedback on why
this is a good/bad idea.

Cheers!

[Jamie Neil]

Andrew Short wrote:
> I'm sure that others will disagree with the various points I've made,
> so I would really appreciate it if I could get any feedback on why
> this is a good/bad idea.

Gets my vote, although I prefer Ag_ ;)

Jamie

[Ingo Schommer]

Keep in mind that this will heavily affect user and dev documentation,
which is currently not specific to a version (more on that soon,
we hope to change that). And it will invalidate pretty
much all code examples from the book thats coming out
this week *cringe*...

[Cam Spiers]

I think this would be so awesome :)

HTTP was a big issue for me in one particular project.

[Sam Minnee]

> Keep in mind that this will heavily affect user and dev documentation,
> which is currently not specific to a version (more on that soon,
> we hope to change that).

Yeah, that particular lock-in is a bit of a liability.

> And it will invalidate pretty
> much all code examples from the book thats coming out
> this week *cringe*...

That would be unfortunate. It's also an indication of the backward
compatibility issues that it would create.

Initially, a clear need for renaming the SS_HTTPResponse,
SS_HTTPRequest, SS_Query, SS_Database was identified. Obviously, if
we were starting from scratch it would be nice to rename other classes
to SS_Controller, SS_Session, etc.

However, given that there aren't actually any known problems with
these classes currently, is it really worth the backward compatibility
nightmare?

I would probably suggest that the "SS_" prefix is the most
appropriate, but that we limit the renaming of classes only to those
with known issues:

SS_HTTPResponse, SS_HTTPRequest, SS_Query, SS_Database

Are there others?

[Ingo Schommer]

(Doh, this mssage should've been here a bit earlier, sent from the
wrong email address. Some of the mentioned points have already been
discussed since).

For reference, here's a previous discussion on this topic?
http://groups.google.com/group/silverstripe-dev/browse_thread/thread/62069368c1a3d4df/9f9152e77060c394?lnk=gst&q=namespace

On 1/10/2009, at 9:23 PM, Andrew Short wrote:

>
> Hey All,
>
> I'm currently creating a class as part of the nestedurls branch -
> however due to it's possibly conflicting name I will be namespacing it
> in order to avoid problems such as that mentioned in issue #2827.
> Indeed, I think it is a good idea to namespace several core classes
> which are likely to conflict, either due to code from the PHP core/
> extensions or from other frameworks. Classes such as Session,
> Controller and Int are a few I can think of off the top of my head
> that, in my opinion, are likely to conflict.

I think its mainly a timing issue, as we can namespace properly
in 5.3. Now, can we live with potential naming conflicts
until 5.3 is a minimum requirement? Not sure, its still a long
way to broad 5.3 support...

>
> However, the current practice of namespacing core classes with "SS"
> when a conflict found should in my opinion be changed to proactively
> namespacing commonly named classes with an "SS_" prefix,

Agreed, the underscore makes it easier to read.

> * I think we should proactively namespace common class names in order
> to prevent issues recurring in the future, as well as easing
> interoperability between sapphire and other scripts/components.

I'm particularly worried about thirdparty modules using
very generic names that are likely to clash.

>
> However, the main problem with changing namespace separator and adding
> namespaces is backwards compatibility - we would have to rename
> SSDatetime to SS_Datetime for consistency, and I would recommend
> namespacing most of the DB field types due to their common names.

I'm not really a fan of "namespacing most classes",
we have to stay consistent. I know we're already violating
that with SSDatetime, but that was an urgent exception which
would conflict with every PHP 5.2 installation out there.

> In order to address this, I have created a very basic prototype of a
> refactoring script that can replace references to any namespaced
> classes - and as such I would recommend that as part of 2.4 we
> identify and namespace potential problems, as well as distribute a
> separate CLI (or web based task) utility to rename namespaced classes,
> in order to ease the transition for people upgrading to 2.4.

Good idea, we've got a lot of code to migrate.
One of the major downsides is that we lose backwards compatibility
for modules, we'd have to maintain two release branches for each
of them for a while (2.3 and 2.4 compatible). The switch to ANSI SQL
quoting makes this a pain already, so it might actually be good
to do in the same go.

Andrew, in case we decide to go this way, how much development
help can you provide? I think part of the reason we didnt do this in
the
past is that its basically a high impact distraction from actually
building stuff ;)

Ingo

[Hamish Campbell]

On the flip side:

Given that PHP 5.3 includes namespaces that will solve this problem,
is it really necessary? Can we 'make do' until such a time as real
namespaces are common?

Lets look at the cases where there are collisions. Generally, this
arises in shared hosting environments where apache has pre-loaded
classes for other applications (or just because that is how they were
configured). Either you are not able to change the configuration, or
the other applications are reliant. Since I'm not aware of any
collisions with any significant mainstream modules (correct me if I'm
wrong), it seems pretty rare and doesn't effect most people using it
in a serious production environment.

That is not to belittle those of us using it in not-so-serious
environments, but the impact to date has been minimal and relatively
simple to work around. I produced a patch for the above classes listed
in about 15 minutes - not exactly the end-of-the-world, and it would
be quite possible to produce patches for the major releases for people
who needed it. Or, even, a tag for a "namespaced" version.

Obviously namespaces add a dependency for PHP 5.3, which hasn't been
released yet and (judging from the time it took for most to upgrade
from 4 to 5) won't be mainstream for a while. However, it seems a
shame to go this long, only to change to a 'psuedo-namespace' and set
ourselves up for a future of: \SilverStripe\SS_Session()

Regards,
Hamish

[Hamish Campbell]

Ah, right, what Ingo said. :P

[Sam Minnee]

> Given that PHP 5.3 includes namespaces that will solve this problem,
> is it really necessary? Can we 'make do' until such a time as real
> namespaces are common?

Bear in mind that PHP 5.3 is unlikely to have a widespread install-
base for at least another 6-12 months.

> Since I'm not aware of any
> collisions with any significant mainstream modules (correct me if I'm
> wrong), it seems pretty rare and doesn't effect most people using it
> in a serious production environment.

There's a reasonably popular PECL extension that provides HTTPRequest
and Query classes. See http://open.silverstripe.com/ticket/2578 for
more information.

> Obviously namespaces add a dependency for PHP 5.3, which hasn't been
> released yet and (judging from the time it took for most to upgrade
> from 4 to 5) won't be mainstream for a while. However, it seems a
> shame to go this long, only to change to a 'psuedo-namespace' and set
> ourselves up for a future of: \SilverStripe\SS_Session()

Agreed, so in my mind, "making do" would mean renaming
SS_HTTPResponse, SS_HTTPRequest, SS_Query, SS_Database. These classes
aren't instantiated that commonly in project code, so the backward-
compat issues are minimal.

Namespacing is something that we can look at in the future, but we're
not going to be able to drop support for PHP 5.2 any time soon, so
we'd need to work out a way of dealing with that, possibly by having a
build script that transformed the main codebase into something
compatible with PHP 5.2.

So, can we reframe this discussion to be a little more specific:
arguments for and against renaming just these 4 classes:
SS_HTTPResponse, SS_HTTPRequest, SS_Query, SS_Database

[dalesaurus]

While namespaces are an exciting new feature to PHP developers that
have been long on envy towards other languages, I agree that the best
path would be to 'make do' for now. As noted previously widespread
5.3 installation is a ways off (Think of Redhat alone...).

I think it would be better to have a patch or branch for 5.3
compatibility, with the generally available version being broadly
compatible with standard <5.3 PHP environments. Allow those with the
means and tools to test/build/improve the namespaced branch to do so,
at will.

For triage in the meantime the measure involving "Renaming just these

4 classes: SS_HTTPResponse, SS_HTTPRequest, SS_Query, SS_Database"

seems best. This is something that can be feasibly accomplished with
upgrade scripts for existing codebases and has the added benefit of
moving the mindset towards namespaced code.

PS. A nod towards Jamie with the Ag_ suggestion, it has some nerdy
earth science appeal. This year is the 140th anniversary of the
Periodic Table! I still like SS_ as the first choice though.


"Making Do" meaning:

> Agreed, so in my mind, "making do" would mean renaming
> SS_HTTPResponse, SS_HTTPRequest, SS_Query, SS_Database. These classes
> aren't instantiated that commonly in project code, so the backward-
> compat issues are minimal.

[Sigurd Magnusson]

Hamish - Just to point out that PHP 5.3 came out in June, but as you
say, it will take a long time for the series to become mainsteam.
Sig.

[Sam Minnee]

Yeah, I should point out that right now we still have PHP 5.1 as our
minimum requirement, because RedHat Enterprise Linux still doesn't
ship with PHP 5.2, and many larger installations run on RHEL. We have
to focus on the needs of our users.

[Morven Lewis-Everley]

I find it quite annoying that RHEL is still running on 5.1.

Luckily our hosting company is kind enough to provide us with an updated version, if we ask, but it seems quite odd that they haven't updated to at least 5.2 yet :(.

Mo

[Michael Mitchell]

Its also worth noting that cPanels EasyApache does not support 5.3 yet either which means a lot of servers running whm/cpanel wont have 5.3 easily available to them either. I'm not considering upgrading my servers to 5.3 until it is available through EasyApache.

Regards, Michael Mitchell

Phone: 03 453 4091
Mobile: 021 280 4589
Skype: michaelmitchell.co.nz
PO Box 13035 - Green Island - Dunedin

[Sam Minnee]

> So, can we reframe this discussion to be a little more specific:
> arguments for and against renaming just these 4 classes:
> SS_HTTPResponse, SS_HTTPRequest, SS_Query, SS_Database

To bring closure to this thread; these changes have been implemented
in core.

http://open.silverstripe.org/changeset/90075
http://open.silverstripe.org/changeset/90076

An additional change has been included in this: classes that were
previously namespaced with "SS" have been renamed to be namespaced
with "SS_".

[Cam Spiers]

Thanks Sam!