Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#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 Olly Betts, 16 years ago

Resolution: wontfix
Status: newclosed

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.

comment:2 by Ivo Jansch, 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 Olly Betts, 16 years ago

http://www.swig.org/

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...

comment:4 by Ivo Jansch, 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).

in reply to:  4 comment:5 by James Aylett, 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 Ivo Jansch, 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 James Aylett, 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).

Note: See TracTickets for help on using tickets.