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

Colin Clark colin.clark at
Mon Jul 6 22:09:38 UTC 2009

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 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,


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

More information about the fluid-work mailing list