Code for review svn commit: 7499 and svn commit: 7500

[Colin Clark]

Hi Laurel,

I've take a look at the commits you mentioned below. I'd like Jacob to  
take a look at them when he gets a chance, too.

It's really nice to see the Builder server-side coding coming along  
well. A couple of quick comments and questions...

I can't quite tell why you've structured your BuilderUtilityClass as a  
full-fledged class with instance methods. There seems to be no state  
here, just a couple of standalone utility functions. Indeed the naming  
suggests as much--it's not an object with any real identity, just a  
collection of functions. Maybe it's easier to ditch the class and just  
invoke these functions directly? Or perhaps there's a bit more  
thinking about how to structure the various bits of logic in your code?

You've got the list of available modules hardcoded at the moment, but  
I notice that you've got a comment in the code saying that this will  
change later. How will you determine these module values? From parsing  
Infusion's Ant build.properties file?

I notice that you always return true from your processPostVariables()  
method, but then this value is checked in case any errors occurred.  
Looks like a bug to me, or perhaps something you haven't had a chance  
to implement yet.

$successPost = processPostVariables();
if (!$successPost)
{
     returnError("Cannot process input variables");
     exit (1);
}

In postProcessor.php, I notice that you define $minified and $includes  
as global variables. How come? If it's necessary, is there a way you  
can define a class instead that will store this state and the methods  
that depend on it without requiring globals?

Hope this helps,

Colin

On 3-Jul-09, at 12:41 PM, Laurel A. Williams wrote:
> The commits are more work on the postProcessor code for the custom  
> builder. For FLUID-2977, I re-wrote the processPostVariables  
> function because the data posted has changed considerably since I  
> first wrote the code. Hopefully we've finalized it now. I also  
> refactored out some utility functions used by processPostVariables  
> into a separate class (BuilderUtilityClass) and created tests for  
> those functions (FLUID-2952).
>
> Can someone review these please. Thx.
>
> Laurel

---
Colin Clark
Technical Lead, Fluid Project
Adaptive Technology Resource Centre, University of Toronto
http://fluidproject.org