Looking at this sample function:

function luceneapi_mymodule_facet_handler() {
  // gets facet value passed by user
  $location = luceneapi_facet_value('location', '');

  // returns subquery to filter by location, in this case it is a wildcard query
  // NOTE: see luceneapi.query.inc for available query types and parameter definitions
  return luceneapi_query_get('wildcard', $location, 'location');
}

it's not at all evident how luceneapi_facet_value() figures out the return value. There's no other parameter than a keyword, so it looks like variable_get(), but we already have variable_get().

As it turns out, the answer is that 'location' is a key in $_GET, so the function is the interface between the (hijacked...) search form and the module, which is an important thing to know, but that's not even guessable from the function name. This gets even more confusing when we notice luceneapi_query_get() further down...

Not sure what the answer is, but it was a minor stumble for me (assuming I got this right). There doesn't seem to be a Drupal naming convention, and $_GET is accessed directly all over core...

Comments

cpliakas’s picture

cpliakas commented

If it doesn't seem evident to you, then I will look at it. I am deep into the code, so I have really internalized the function names making it hard to objectivly determine whether the name is transparent or not. If you have any suggestions as to what this function should be named, please let me know.

The functions simply acts as a wrapper around $_GET as you mentioned. The reason this function exists is that it eliminates the need to manually check whether the key was passed in two separate locations. When you define a facet, you generally have to retrieve the value from $_GET in two places. One is in the callback where you have to get the value passed by the user to turn it into a query object, and the second is in the #default_value selector of the facet's FAPI array. Instead of having to manually check whether the array key exists in $_GET and assigning a default value if it does not, luceneapi_facet_value() does that for you so you can DRY up your code. Like variable_get(), it also allows you define a default value if the key does not exist. As Drupal moves towards E_STRICT mode, it will hopefully eliminate any "undefined index" errors by attempting to access a value that was not passed by the user.

Another reason for this function is something I actually forgot about until now. If you pass NULL as the second parameter, it returns the entire $_GET array with 'q' stripped off (thanks to ebeyrent for that idea). The function is incomplete as it should actually strip 'page', 'lucenesort', and maybe some other keys that should never be used as the name of facet form elements. I may also play around with getting a whitelist of facets defined by all the hook_facet() implementations and only returning those. This way you can truly get all the facets passed by the user.

I am also in the process of refactoring the entire facet section. There is a cool module idea from webkenny called Search Lucene UI, but I will need to take some steps for him to accomplish what he wants to do. I have changes a lot of the function names to follow the "module"-"noun"-"verb" convention (for the most part), so luceneapi_facet_value() will most definitely turn into luceneapi_facet_value_get().

Thanks again,
Chris

shunting’s picture

shunting commented

I think module-noun-verb should do it, and if there's a reasonably rigorous set of nouns and verbs, that might go some way toward self-documentation....

cpliakas’s picture

cpliakas commented
Status: Active » Fixed

Function names have been normalized, the function mentioned above has been renamed to luceneapi_facet_value_get().

shunting’s picture

shunting commented

Cool. Thanks so much!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.