Services API misuses hash/number (#) symbols
| Project: | Services |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
The services API is littered with completely unnecessary hash symbols. Some properties have hashes, some don't, but there's absolutely no reasoning to which ones have hashes or when/why they're necessary.
It looks like the original API was meant to build on the properties of Form API, but without understanding why FAPI uses hashes. FAPI uses them because it doesn't have a defined structure. Sometimes an array is a form element, sometimes it's a property. Properties are designated with the hash sign.
In the case of Services, it's all properties, just like a lot of other hooks such as hook_menu(), hook_views_default_views(), or hook_action_info(). So services is including these hash symbols for absolutely no reason whatsoever.
This patch applies to the D6 1.x branch (sorry I didn't realize there was a 2.x one, I'd be happy to reroll), and removes all these hash symbols from the API. However to keep it down on size, I did not actually update any of the implementations. Instead, the hash signs are "deprecated" (optional basically) until we can get around to removing them from all the implementations.
| Attachment | Size |
|---|---|
| services_hashes.patch | 13.97 KB |
#1
Agreed. If you have control over an API's data structures, implementing something like 'children' as a defined element key is possible when nesting is required. Hash prefixes should be used for drupal_render()able structures, but are unnecessary for this kind of system.
#2
I've been updating Flag to use Services (with Amitaibu), and it turns out Flag seems targeted towards the 2.x version of Services. Here's an updated patch for 2.x (not extensively tested). It seems like this problem has just gotten worse in the 2.x version with more hooks and more properties.
#3
This version includes coder issues fixed, I've tested manually and seems to work fine, at least the services are still there! ;)
Good patch.. I'll start porting to d7
#4
Sorry, two nasty + characters in the previous patch. Seems that heyrocker also ported the patch to d7, so just in case someone wants to review I've included here.
#5
Committed, thanks! Pushing the version number to make sure we forward-port this to the D7 port.