Josh,
AFAIK the function should not use $context. Filling $context with information is IMO a Controller task. I'd prefer to use an internal variable e.g. $membergroups and return that var. Please see https://github.com/elkarte/Elkarte/wiki/Architecture

Please keep in mind: there are many different cases, where certain groups need to be exculded (e.g. admin/local moderators / protected groups / hidden groups ...) from the list or you just want to pick some specific groups. There's already a function which covers most cases: getBasicMembergroupData()

Another global variable sucks.

Excluding groups is easy. I've done it in a lot of cases.

I think you misunderstood me..There's no need to globalize anything.
What I meant is basically this:

function loadMemberGroups()
{
$membergroups = array();
/** query to fill $membergroups with data; **/
return $membergroups;
}

$context['membergroups'] = loadMemberGroups();

I thought about that but I wanted the array to persist throughout the life of the script. Otherwise it would be doing a lot of calls to that. Or, if you did it that way, you would most likely have to put it early in the execution of the script which might mean more queries than necessary.

I do agree that $context is a bad variable because it should be used to pass from controller to view, but if you moved it, you'd need a lot of code duplication and that code duplication would need to be done in the models anyway.

I thought about that but I wanted the array to persist throughout the life of the script. Otherwise it would be doing a lot of calls to that.

Why? You have the same calls to the membergroups() function. (or, could you show a counter-example?)

I don't understand your question or statement.

The execution of the script goes (ideally) like this:

• dispatching

  -> action_something() method

  • which uses subs1, subs2, subs3
    • and sends to template.

Where do you see "a lot of calls to that"? The action_something() method is a single method (sometimes two), so after the call to getMembergroups(), it has the result, likely in $context. The template has $context.
I think in most cases the execution of the script with core files alone has a single call to getMembergroups().
With addons and integrated sites, it may have more calls for their own needs, but that's a good thing, not a bad thing. Sharing data through $context with external apps isn't cool, nor sharing internally for that matter. The API exists to be used.

That's fine. Then the controller needs to have a persistent method to hold
on to things like that.

On Fri, May 10, 2013 at 6:40 PM, Norv notifications@github.com wrote:

In sources/Load.php:

@@ -2672,3 +2686,39 @@ function determineAvatar($profile, $max_avatar_width, $max_avatar_height)

return $avatar;

}
+
+/**

• * Load all of the membergroups
• * @global array $smcFunc
• * @global array $context
• * @global array $modSettings
• */
  +function loadMemberGroups($force = false)

The execution of the script goes (ideally) like this:

• dispatching -> action_something() method
  • which uses subs1, subs2, subs3
  • and sends to template. Where do you see "a lot of calls to that"?
    The action_something() method is a single method (sometimes two), so after
    the call to getMembergroups(), it has the result, likely in $context. The
    template has $context. I think in most cases the execution of the script
    with core files alone has a single call to getMembergroups(). With addons
    and integrated sites, it may have more calls for their own needs, but
    that's a good thing, not a bad thing. Sharing data through $context with
    external apps isn't cool, nor sharing internally for that matter. The API
    exists to be used.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/380/files#r4180845
.

It likely doesn't need anything more. You made it work yourself, without anything more 'persistent':
https://github.com/joshuaadickerson/NotElkarte/commit/c5a2928c63a3b81a1f93541648c3544666a05acd
// grab $topic_info
// use something from it
// continue
// use something else from it
$topic_info is just a local variable in the controller function. That's fine by the looks of it. Whatever the getTopicInfo() does, it's a blackbox, all the controller needs is its result.