#372 closed defect (wontfix)
Function naming conventions in php binding
Reported by: | Ivo Jansch | Owned by: | Olly Betts |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Xapian-bindings | Version: | |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Operating System: | Linux |
Description
The php extension adds functions to php such as 'new_WritableDatabase', 'new_Document' and 'auto_open_stub'.
While this is probably a 1:1 mapping to Xapian's C api, it doesn't adhere to php coding standards, and might lead to conflicts in the future.
Suggestion: lowercase and prefix all functions, like this:
new_WritableDatabase => xapian_create_writabledatabase new_Document => xapian_create_document auto_open_stub => xapian_auto_open_stub
etc.
Change History (7)
comment:1 by , 16 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 16 years ago
I'm not sure what swig is, but I do know that about 80% of the php extensions uses prefixes. It's not only to prevent clashes, it also is a lot clearer as you can immediately see that the function that's being called in the code is a xapian one.
comment:3 by , 16 years ago
SWIG automatically generates all that code for us. If it should look different, then the changes need to be made to SWIG (or in a few cases to how we call SWIG).
From Xapian's point of view, saying you don't like the code SWIG generates for our PHP bindings is kind of like complaining to us that you don't like the machine code GCC generates from Xapian's C++ source code. Even if we agree, the place to fix it probably isn't here...
follow-up: 5 comment:4 by , 16 years ago
That's actually an interesting discussion. I use xapian from a php programmer's point of view. That Xapian uses a tool to develop the php extension is a good thing, but I'm sure Swig supports some kind of function name translation/prefixing. From a programmer's point of view though, consistency in naming is more important than the underlying tools used.
As an example, see the ones in PECL, e.g. for Swish and Sphynx:
http://pecl.php.net/packages.php?catpid=57&catname=Search+Engine
Consistency would greatly help adoption in the php community (as would a pecl extension but that's a different story).
comment:5 by , 16 years ago
You're talking about the PHP4-style code, aren't you? Is there a reason you're not using the PHP5-style object orientated interface? There are probably a few grungy bits left there which we could perhaps look at, but my understanding is that it's pretty close to what's expected.
Note that we cannot submit Xapian's PHP bindings to PECL, because Xapian is licensed under the GPL, which PECL explicitly does not allow.
comment:6 by , 16 years ago
No I'm talking about the actual extension (xapian.so), which the php5 style xapian.php uses under the hood.
comment:7 by , 16 years ago
Okay, but that isn't really intended to be user-exposed so (beyond the risk of symbol conflicts) I don't really see the problem. Certainly consistency doesn't need to apply to internal systems (yes, it'd be nice if it did, but that's somewhat a different argument).
Function names are case insensitive in PHP (e.g. try
php -r 'pRiNT "hello";'
so forcing them to lowercase wouldn't help avoid a collision.These functions are purely an implementation detail for how SWIG implements PHP5 class wrappers. The risk of a clash seems to be purely theoretical as far as I can see, so sorry, but I'm going to mark this as "wontfix". If there ever was an issue, we could patch SWIG to add a prefix and not break any code using the bindings.