Refactor away Mahara's "Frankenname"-formula API's

Bug #1194373 reported by Aaron Wells
This bug report is a duplicate of:  Bug #1576075: Namespaces and Autoloader. Edit Remove
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Triaged
Wishlist
Unassigned

Bug Description

"Magic strings" are widely considered to be a suboptimal coding practice, and I'm of the opinion that functions and classnames that follow a specified naming formula, are a variant on this. That is to say, the names of things should be stored in the programming language's symbol table in some way, and arrived at by manipulating entries in the symbol table; not by taking string constants and combining them and checking if they match up with an entry in the symbol table.

So, I'd like to see the following happen:

1. API's that are based on checking for functions with specific predefined names, should be replaced with API's based around subclasses or interface implementations. Pieforms is the big one I'm looking at here. When you create a new Pieforms element type, you create a series of functions that follow specific naming patterns: pieform_element_{elementname}_{functionname}. The proper, modern way to do this would be to define a parent Pieform element class (say Pieform\Element) or interface, which specifies all the functions that a pieform element is expected to have, and then make all implement that.

2. API's that are based on checking for the existence of a class that follows a specific naming pattern, should be changed to looking for classes in a specific PHP namespace. For instance, PluginSearchElasticsearch should become Plugin\Search\Elasticsearch.

However, if we do this, we should bear in mind the following:

1. It probably won't have any visible benefit at all to end-users. :( So don't make it too high a priority.

2. We should, if possible, make code backwards-compatible. So, while using the new API, we should check for plugins that are still built around the old API. It might even make sense to add e.g. classloaders that look for the old Frankenname formulas and automatically map them to the newer namespace formulas (and something similar for function names, if possible).

Tags: api
Revision history for this message
Aaron Wells (u-aaronw) wrote :

Actually, after testing out a conversion of Mikael's ElasticSearch plugin to use Namespaces, I'm not convinced they're a good idea just yet. The main problem is that namespaces aren't first-class objects. So if you're instantiating or referencing classes dynamically, you still wind up using string constants. You just have stuff like $class = "\\ES\\Type\\{$type}" instead of $class = "ES_Type_{$type}". Which, really, isn't much of an improvement.

This article sums up some of the other current issues with PHP namespaces: http://pornel.net/phpns

I don't think namespaces are necessarily bad, just that we don't need to rework our plugin API to require them, until there's a bit more language support.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.