[jira] Resolved: (SHINDIG-351) Featureloader legibility of register recursion / Bugs in register recursion

["Chris Chabot (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SHINDIG-351?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chris Chabot resolved SHINDIG-351.
----------------------------------

    Resolution: Fixed
      Assignee: Chris Chabot

The feature loader is known to be a bit messy, the basis of PHP shindig was based on java's structure, and that was known to be a bit messy due to the haste in getting something out there. Nice cleanup, thanks

Applied and commited

> Featureloader legibility of register recursion / Bugs in register recursion
> ---------------------------------------------------------------------------
>
>                 Key: SHINDIG-351
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-351
>             Project: Shindig
>          Issue Type: Bug
>          Components: Gadget Rendering Server (PHP)
>         Environment: Lighttpd, Mac OS X (should aplly to all)
>            Reporter: Karsten Beyer
>            Assignee: Chris Chabot
>            Priority: Minor
>         Attachments: fix-issue-SHINDIG-351_2.patch
>
>
> The variable names in the JSFeatureLoader.php ->loadFeatures and JSFeatureLoader->register methods are very confusing. The same item has different names in different methods.
> Also there were two logical bugs in the code. 
> First the loadFiles method gets the $deps (<- confusing name, as this holds in fact all features, and no dependencies at all) as a reference. Nevertheless this is the call:
> $deps = $this->loadFiles($path, $deps);
> and in loadFiles: return $features;
> This is unnecessary as loadFiles is anyway working on a reference of this object.
> The second logical bug is with the $registered array in the register method. First of all, the value is set, but the key is checked:
> if (isset($registered[$feature->name])) {
> ...
> $registered[] = $feature->name;
> which means that this is not considered at all and all features are registered multiple times (which is later no big problem, as they are overwritten, but unnecessary nevertheless). The second issue is that the $registered array is not passed as reference, which means that it stays empty during the recursion and does not work at all.
> I have produced a patch that fixes all these issues with the loadFeatures functionality (including some changes in the locally used variable names to make it easier to understand what is going on), which i will attach in a few minutes..

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.