FLUID-2953

[Justin]

Hello Laurel,

I just took a quick look at your latest commits.

Looks good. Thanks for fixing up combineArrays.

There are a few issues that you can spot by doing some linting,  
including: missing semicolon, whitespace issue, and some unused  
variables.

- Justin
On 24-Jul-09, at 1:25 PM, Justin wrote:

> 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
>
> _______________________________________________________
> 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/20090727/c572f9d0/attachment.htm>