FLUID-2953

[Justin]

Hello Laurel,

Here is some feedback.


General
=======

Could use some linting
some whitespace issues
missing some ";"


checkRenderedModules
====================

var i is declared twice, once in each for loop
numModuleValues is used before it is declared
the string ".flc-customBuild-module input:checkbox" is used twice. You  
could probably declare renderedModuleCheckboxes earlier and then use  
it in numRenderedCheckboxes
$.unique only works for DOM elements not strings ( http://docs.jquery.com/Utilities/jQuery.unique 
  )
you can use the hasSameValues function that i made, which tests if two  
arrays are the same size and have the same values


customBuildTester
===============

Only takes in one set of test data
there are lots of possible ways to solve this
pass in more data sets (via array or parameters)
move the helper functions out of the custBuildTester function and have  
multiples of these types of functions that get called in the $ 
(document).ready function
etc.

hasSameValues
=============

Okay so this is something I wrote, but thought you might want to fix  
it, or I can it's up to you.

currently it checks one array against another, but you could get a  
situation like this [one, one], [one, two] That could pass, since one  
is in both arrays. So the if condition should contain a test for  
checking both arrays against each other. The if statement would have  
something like this     $.inArray(arrayOne[i], arrayTwo) === -1 ||   
$.inArray(arrayTwo[i], arrayOne) === -1
On 23-Jul-09, at 3:18 PM, Laurel A. Williams wrote:

> Hi all,
>
> I've spent the last day or two working with the tests for the  
> customBuild component - specifically the tests that check if the  
> rendering is working as expected. I've added a little code, many  
> comments and refactored the tests to improve readability (for me  
> anyway) and to chunk the code into logical groupings (again, in my  
> opinion). I'm excited that I'm actually doing javascript programming  
> after 7 months on FLUID <grin>!!
>
> I think it is probably time to ask for a quick review of my efforts,  
> before I go too much farther. Looking forward to your feedback.
>
> Laurel
>
> -- 
> Laurel A. Williams
> Adaptive Technology Resource Centre
> University of Toronto
>
> _______________________________________________________
> fluid-work mailing list - fluid-work at fluidproject.org
> To unsubscribe, change settings or access archives,
> see http://fluidproject.org/mailman/listinfo/fluid-work

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.idrc.ocad.ca/pipermail/fluid-work/attachments/20090724/6e0f4485/attachment.htm>