Allow multiple values in a single variable_set() call

I just had another idea in that vein: instead of registering variables with yet another hook, if should be possible to extend the syntax of variable_set() beyond what you just did. Instead of having it variable_set($name, $value = NULL) why not go further and accept variable_set($name, $value = NULL, $default = NULL). This would set the default if the third argument were present, or leave it unchanged otherwise. And maybe while we're at it, add the long-talked-about fourth argument defining prefetching to solve the problem outlined by DWW, and which other modules like chatroom have.

From there, variable_get() could also be modified like variable_get($name, $default = NULL). And using variable_get($name) would now use the default defined upon variable_set'ting, still leaving the current syntax available, to serve two purposes: backwards compatibility, and ability to change defaults on the fly without otherwise affecting the stored default.

I tend to think this is more elegant than adding a hook_variables.

What do you think of it ?

I have to agree with the fact that it is slightly OT. But not with the other claims, especially the fact that "you can never guarantee": such variables should probably always be created in the .install file anyway, thus ensuring that they have a value at "run" time, while code running not running after the page full bootstrap process, could still use the second param in variable_get(). It would really reduce redundant code in a lot of places, and would still get us the same benefits as some hook_variables variant without the additional API clutter of yet one more core hook.

But you're right, your suggestion does not have to touch these larger issues, and can remain "atomic" as you suggested it.

Lovely patch. Perhaps I am paranoid, but I would like to see a stronger 'access arguments' for the test menu item $items['system-test/variable-get']. i could imagine that this module get stuck enabled on someone's site and now their variables are exposed. After that, this needs a verify that the tests pass and then RTBC.

In the test, create a user that has 'access administration pages' and login as that user. then you can increase the access arguments.

http://drupal.org/node/138544#comment-1129772

Crell -

Since you've been helping so much on the sister issue #138544: Allow multiple variable deletes in variable_del(), I thought I would re-roll this against HEAD to help out.

I've reviewed the code and looks very solid. Even includes a test case (though, it needs some more testing according to @5,@6,@7).

Josh

Re-rolled against latest HEAD.

-

Yup.

see also #975118: Allow variable functions (variable_del, variable_get, variable_set) to receive an array of name/value pairs

here is patch from #975118: Allow variable functions (variable_del, variable_get, variable_set) to receive an array of name/value pairs

• Fixed some of the code comments
• Code formatting
• Made variable_del() use a single execute() statement via a db_or()
• Fixed some of the tests -- Not sure why they're still failing though
• Moved to Drupal 8

@catch directed me to this issue from #987768: [PP-1] Optimize variable caching. In that issue, progress is being made toward building a cached incremental variables array. Part of that refactoring effort involves moving a data base call to variable_get() in the event of a missing $conf[] entry.

Reading the patch there led me to raise the question whether we did not want to introduce a variable helper function that bulk loads a limited number of variables from the {variables} table to $conf[] and cache based either on variable name pattern or a list of variable names. I similarly raised the thought that we should introduce bulk operations with functions like variables_set(array) and variables_del(array) [note plural format of variable].

I am not tied to idea of new plural functions, but thought it might make sense for explicitly different function calling patterns in the case of bulk operations. I am repeating those thoughts here with a hope to push along a solution for bulk variable set()/del() calls.

I think using separate functions sounds like a good idea, we've been trying to do similar things elsewhere too.

However, my naming suggestion would be variable_(get|set|del)_multiple(), then it is very similar to entity load functions for example and IMHO easier to find than with a different prefix.

Just like node_load() and node_load_multiple(), variable_get() could then simply be a wrapper function around variable_get_multiple().

Grrrrr, in almost every other language, you can do things like:

/**
 * Variable set for multiple values.
 */
function variable_set(array $variables = array()) {
  // Set multiple variables.
}

/**
 * Variable set for a single value.
 */
function variable_set(string $variable, $value) {
  // Set the single variable.
}

I miss strict languages... As you can tell, I'm still against having multiple functions doing the same thing. #D8DX, please!

After posting #19 above, I realized most of the discussion about function call format has occurred over in #138544: Allow multiple variable deletes in variable_del(). May I suggest that we focus the discussion about the function call patterns over there?

Then we can return here to implement whatever is agreed upon for the variable set() and del() functions. The proposed implementation of the variable_set() function has been broken off into #317930: Allow multiple values in a single variable_set() call and needs to be part of the function profile discussion as well.

+1 for variable_(get|set|del)_multiple() pattern as it would match many other Drupal calling patterns.

Edit: Just realized this was the multiple variable_get() thread... not the multiple set()/get() branch. Main thought about define function profiles in #138544: Allow multiple variable deletes in variable_del() still applies.

Sorry cross-post changed issue tags ...

Setting as 'Won't Fix' as this is no longer really applicable in light of the CMI changes.