Refactoring GST.

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/graph.js

@@ -141,25 +141,18 @@ define('Graph', ['logme'], function (logme) {
                 'end': 10,
                 'step': 0.1
             };
-            logme('Default xrange:', xrange);
 
             // The 'xrange' is a string containing two floating point numbers
             // separated by a comma. The first number is the starting
             // x-coordinate , the second number is the ending x-coordinate
             if (typeof config.plot['xrange'] === 'string') {
-                logme('xrange is a string; xrange = "' + config.plot['xrange'] + '".');
-
                 xRangeStr = config.plot['xrange'];
                 xRangeBlobs = xRangeStr.split(',');
 
                 if (xRangeBlobs.length === 2) {
-                    logme('xrange contains 2 blobs; 1 -> "' + xRangeBlobs[0] + '", 2 -> "' + xRangeBlobs[1] + '".');
-
                     tempNum = parseFloat(xRangeBlobs[0]);
                     if (isNaN(tempNum) === false) {
                         xrange.start = tempNum;
-
-                        logme('First blob was parsed as a float. xrange.start = "' + xrange.start + '".');
                     } else {
                         logme('ERROR: First blob was parsed as a NaN.');
                     }
@@ -167,8 +160,6 @@ define('Graph', ['logme'], function (logme) {
                     tempNum = parseFloat(xRangeBlobs[1]);
                     if (isNaN(tempNum) === false) {
                         xrange.end = tempNum;
-
-                        logme('Second blob was parsed as a float. xrange.end = "' + xrange.end + '".');
                     } else {
                         logme('ERROR: Second blob was parsed as a NaN.');
                     }
@@ -176,8 +167,6 @@ define('Graph', ['logme'], function (logme) {
                     if (xrange.start >= xrange.end) {
                         xrange.start = 0;
                         xrange.end = 10;
-
-                        logme('xrange.start is greater than xrange.end - will set defaults. xrange.start = "' + xrange.start + '". xrange.end = "' + xrange.end + '".');
                     }
 
                 } else {
@@ -191,17 +180,13 @@ define('Graph', ['logme'], function (logme) {
             // we will use it to generate a 'step' - i.e. the distance (on
             // x-axis) between two adjacent points.
             if (typeof config.plot['num_points'] === 'string') {
-                logme('num_points is a string. num_points = "' + config.plot['num_points'] + '".');
-
                 tempNum = parseInt(config.plot['num_points'], 10);
                 if (
                     (isNaN(tempNum) === false) ||
                     (tempNum >= 2) &&
                     (tempNum <= 500)
                 ) {
-                    logme('num_points was parsed as a number. num_points = "' + tempNum + '".');
                     xrange.step = (xrange.end - xrange.start) / (tempNum - 1);
-                    logme('xrange.step = "' + xrange.step + '".');
                 } else {
                     logme('ERROR: num_points was not parsed as a number, or num_points < 2, or num_points > 500.');
                 }

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/gst_main.js

@@ -16,23 +16,47 @@ define(
     function GstMain(gstId) {
         var config, gstClass, state;
 
-        // Get the JSON configuration, and parse it, and store as an object.
+        // Get the JSON configuration, parse it, and store as an object.
+        try {
             config = JSON.parse($('#' + gstId + '_json').html()).root;
+        } catch (err) {
+            logme('ERROR: could not parse config JSON.');
+            logme('$("#" + gstId + "_json").html() = ', $('#' + gstId + '_json').html());
+            logme('JSON.parse(...) = ', JSON.parse($('#' + gstId + '_json').html()));
+            logme('config = ', config);
 
+            return;
+        }
+
+        // Get the class name of the GST. All elements are assigned a class
+        // name that is based on the class name of the GST. For example, inputs
+        // are assigned a class name '{GST class name}_input'.
+        if (typeof config['@class'] !== 'string') {
+            logme('ERROR: Could not get the class name of GST.');
+            logme('config["@class"] = ', config['@class']);
+
+            return;
+        }
         gstClass = config['@class'];
-        logme('gstClass: ' + gstClass);
 
-        // Parse the configuration settings for sliders and text inputs, and
-        // extract all of the defined constants (their names along with their
-        // initial values).
-        state = State(gstId, gstClass, config);
+        // Parse the configuration settings for parameters, and store them in a
+        // state object.
+        state = State(gstId, config);
+
+        // It is possible that something goes wrong while extracting parameters
+        // from the JSON config object. In this case, we will not continue.
+        if (state === undefined) {
+            logme('ERROR: The state object was not initialized properly.');
+
+            return;
+        }
 
         // Create the sliders and the text inputs, attaching them to
-        // approriate constants.
-        Sliders(gstId, gstClass, state);
+        // appropriate parameters.
+        Sliders(gstId, state);
         Inputs(gstId, gstClass, state);
 
-        // Configure and display the loop. Attach event for the graph to be
+        // Configure and display the graph. Attach event for the graph to be
         // updated on any change of a slider or a text input.
         Graph(gstId, config, state);
     }

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/inputs.js

@@ -10,14 +10,11 @@ define('Inputs', ['logme'], function (logme) {
 
         allParamNames = state.getAllParameterNames();
 
-        console.log(allParamNames);
-
         for (c1 = 0; c1 < allParamNames.length; c1 += 1) {
             $('#' + gstId).children('.' + gstClass + '_input').each(function (index, value) {
                 var inputDiv, paramName;
 
                 paramName = allParamNames[c1];
-
                 inputDiv = $(value);
 
                 if (paramName === inputDiv.data('var')) {
@@ -29,23 +26,17 @@ define('Inputs', ['logme'], function (logme) {
         return;
 
         function createInput(inputDiv, paramName) {
-            var paramObj, inputWidth, readOnly;
+            var paramObj, readOnly;
 
             paramObj = state.getParamObj(paramName);
 
-            // We will define the width of the slider to a sensible default.
-            inputWidth = 400;
+            // Check that the retrieval went OK.
+            if (paramObj === undefined) {
+                logme('ERROR: Could not get a paramObj for parameter "' + paramName + '".');
 
-            // See if it was specified by the user.
-            if (isFinite(parseInt(inputDiv.data('el_width'))) === true) {
-                inputWidth = parseInt(inputDiv.data('el_width'));
+                return;
             }
 
-            // Set the width of the element.
-            inputDiv.width(inputWidth);
-
-            inputDiv.css('display', 'inline-block');
-
             readOnly = false;
             if (inputDiv.attr('data-el_readonly').toLowerCase() === 'true') {
                 readOnly = true;
@@ -79,9 +70,11 @@ define('Inputs', ['logme'], function (logme) {
                 'outline': 'none',
                 'cursor': 'text',
                 'height': '15px'
-                // 'width': '50px'
             });
 
+            // Tell the parameter object from state that we are attaching a
+            // text input to it. Next time the parameter will be updated with
+            // a new value, tis input will also be updated.
             paramObj.inputDivs.push(inputDiv);
 
             return;

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/sliders.js

@@ -5,7 +5,7 @@
 define('Sliders', ['logme'], function (logme) {
     return Sliders;
 
-    function Sliders(gstId, gstClass, state) {
+    function Sliders(gstId, state) {
         var c1, paramName, allParamNames, sliderDiv;
 
         allParamNames = state.getAllParameterNames();
@@ -13,38 +13,33 @@ define('Sliders', ['logme'], function (logme) {
         for (c1 = 0; c1 < allParamNames.length; c1 += 1) {
             paramName = allParamNames[c1];
 
-            logme('Looking for slider with ID: ' + gstId + '_slider_' + paramName);
             sliderDiv = $('#' + gstId + '_slider_' + paramName);
 
             if (sliderDiv.length === 1) {
-                logme('Found one slider DIV with such an ID.');
                 createSlider(sliderDiv, paramName);
+            } else if (sliderDiv.length > 1) {
+                logme('ERROR: Found more than one slider for the parameter "' + paramName + '".');
+                logme('sliderDiv.length = ', sliderDiv.length);
             } else {
-                logme('Did not find such a slider.');
+                logme('MESSAGE: Did not find a slider for the parameter "' + paramName + '".');
             }
         }
 
         function createSlider(sliderDiv, paramName) {
-            var paramObj, sliderWidth;
+            var paramObj;
 
             paramObj = state.getParamObj(paramName);
 
-            // We will define the width of the slider to a sensible default.
-            sliderWidth = 400;
+            // Check that the retrieval went OK.
+            if (paramObj === undefined) {
+                logme('ERROR: Could not get a paramObj for parameter "' + paramName + '".');
 
-            // See if it was specified by the user.
-            if (isFinite(parseInt(sliderDiv.data('el_width'))) === true) {
-                sliderWidth = parseInt(sliderDiv.data('el_width'));
+                return;
             }
 
-            // Set the width of the element.
-            sliderDiv.width(sliderWidth);
-
-            sliderDiv.css('display', 'inline-block');
-
             // Create a jQuery UI slider from the slider DIV. We will set
             // starting parameters, and will also attach a handler to update
-            // the 'state' on the 'change' event.
+            // the 'state' on the 'slide' event.
             sliderDiv.slider({
                 'min': paramObj.min,
                 'max': paramObj.max,
@@ -55,6 +50,9 @@ define('Sliders', ['logme'], function (logme) {
                 'slide': sliderOnSlide
             });
 
+            // Tell the parameter object stored in state that we have a slider
+            // that is attached to it. Next time when the parameter changes, it
+            // will also update the value of this slider.
             paramObj.sliderDiv = sliderDiv;
 
             return;
@@ -65,7 +63,15 @@ define('Sliders', ['logme'], function (logme) {
             // This will cause the plot to be redrawn each time after the user
             // drags the slider handle and releases it.
             function sliderOnSlide(event, ui) {
-                state.setParameterValue(paramName, ui.value, sliderDiv);
+
+                // Last parameter passed to setParameterValue() will be 'true'
+                // so that the function knows we are a slider, and it can
+                // change the our value back in the case when the new value is
+                // invalid for some reason.
+                if (state.setParameterValue(paramName, ui.value, sliderDiv, true) === undefined) {
+                    logme('ERROR: Could not update the parameter named "' + paramName + '" with the value "' + ui.value + '".');
+                }
+
             }
         }
     }

common/lib/xmodule/xmodule/js/src/graphical_slider_tool/state.js

@@ -6,14 +6,13 @@ define('State', ['logme'], function (logme) {
     // Since there will be (can be) multiple GST on a page, and each will have
     // a separate state, we will create a factory constructor function. The
     // constructor will expect the ID of the DIV with the GST contents, and the
-    // configuration object (parsed from a JSON string). It will return and
+    // configuration object (parsed from a JSON string). It will return an
     // object containing methods to set and get the private state properties.
 
     // This module defines and returns a factory constructor.
     return State;
 
-    // function: State
-    function State(gstId, gstClass, config) {
+    function State(gstId, config) {
         var parameters, allParameterNames, allParameterValues,
             plotDiv;
 
@@ -21,18 +20,20 @@ define('State', ['logme'], function (logme) {
         // an empty object.
         //
         // As we parse the JSON config object, we will add parameters as
-        // named properties (for example
+        // named properties. For example
         //
         //     parameters.a = {...};
         //
-        // for the parameter 'a'.
+        // will be created for the parameter 'a'.
         parameters = {};
 
-        // Check that the required object is available.
-        if (
-            (typeof config.parameters !== 'undefined') &&
-            (typeof config.parameters.param !== 'undefined')
-        ) {
+        // Check that the required parameters config object is available.
+        if ($.isPlainObject(config.parameters) === false) {
+            logme('ERROR: Expected config.parameters to be an object. It is not.');
+            logme('config.parameters = ', config.parameters);
+
+            return;
+        }
 
         // If config.parameters.param is an array, pass it to the processor
         // element by element.
@@ -51,11 +52,18 @@ define('State', ['logme'], function (logme) {
             processParameter(config.parameters.param);
         }
 
+        // If config.parameters.param is some other type, report an error and
+        // do not continue.
+        else {
+            logme('ERROR: config.parameters.param is of an unsupported type.');
+            logme('config.parameters.param = ', config.parameters.param);
+
+            return;
         }
 
         // Instead of building these arrays every time when some component
         // requests them, we will create them in the beginning, and then update
-        // by element when some parameter's value changes.
+        // each element individually when some parameter's value changes.
         //
         // Then we can just return the required array, instead of iterating
         // over all of the properties of the 'parameters' object, and
@@ -63,10 +71,9 @@ define('State', ['logme'], function (logme) {
         allParameterNames = [];
         allParameterValues = [];
 
+        // Populate 'allParameterNames', and 'allParameterValues' with data.
         generateHelperArrays();
 
-        logme(parameters, allParameterNames, allParameterValues);
-
         // The constructor will return an object with methods to operate on
         // it's private properties.
         return {
@@ -91,6 +98,8 @@ define('State', ['logme'], function (logme) {
 
         function getParamObj(paramName) {
             if (parameters.hasOwnProperty(paramName) === false) {
+                logme('ERROR: Object parameters does not have a property named "' + paramName + '".');
+
                 return;
             }
 
@@ -108,6 +117,8 @@ define('State', ['logme'], function (logme) {
             // If the name of the constant is not tracked by state, return an
             // 'undefined' value.
             if (parameters.hasOwnProperty(paramName) === false) {
+                logme('ERROR: Object parameters does not have a property named "' + paramName + '".');
+
                 return;
             }
 
@@ -119,6 +130,7 @@ define('State', ['logme'], function (logme) {
         // Function: setParameterValue(paramName, paramValue, element)
         // --------------------------------------------------
         //
+        //
         // This function can be called from a callback, registered by a slider
         // or a text input, when specific events ('slide' or 'change') are
         // triggered.
@@ -146,12 +158,14 @@ define('State', ['logme'], function (logme) {
         // original value.
         //
         // ####################################################################
-        function setParameterValue(paramName, paramValue, element) {
+        function setParameterValue(paramName, paramValue, element, slider) {
             var paramValueNum, c1;
 
             // If a parameter with the name specified by the 'paramName'
             // parameter is not tracked by state, do not do anything.
             if (parameters.hasOwnProperty(paramName) === false) {
+                logme('ERROR: Object parameters does not have a property named "' + paramName + '".');
+
                 return;
             }
 
@@ -159,38 +173,65 @@ define('State', ['logme'], function (logme) {
             // number.
             paramValueNum = parseFloat(paramValue);
 
-            if (
             // We are interested only in valid float values. NaN, -INF,
             // +INF we will disregard.
-                (isFinite(paramValueNum) === false) ||
+            if (isFinite(paramValueNum) === false) {
+                logme('ERROR: New parameter value is not a floating-point number.');
+                logme('paramValue = ', paramValue);
+
+                return;
+            }
 
             // If the new parameter's value is valid, but lies outised of
-                // the parameter's allowed range, we will also disregard it.
+            // the parameter's allowed range, we will disregard it.
+            //
+            // We will also change the element's value back to the current
+            // parameter's value.
+            if (
                 (paramValueNum < parameters[paramName].min) ||
                 (paramValueNum > parameters[paramName].max)
             ) {
-                // We will also change the element's value back to the current
-                // parameter's value.
+                logme('ERROR: New parameter\'s value lies outside of the allowed range.');
+                logme('Range is: min = ' + parameters[paramName].min + ', max = ' + parameters[paramName].max);
+                logme('paramValueNum = ' + paramValueNum);
+
+                // If element is a slider, the value must be set in a special
+                // way.
+                if (slider === true) {
+                    logme('Changing back the slider.');
+                    element.slider('value', parameters[paramName].value);
+                }
+
+                // If element is a text input, standard jQuery val() function
+                // will work.
+                else {
                     element.val(parameters[paramName].value);
+                }
 
                 return;
             }
 
             parameters[paramName].value = paramValueNum;
 
+            // If we have a plot DIV to work with, tell to update.
             if (plotDiv !== undefined) {
                 plotDiv.trigger('update_plot');
             }
 
+            // Update all text inputs with the new parameter's value.
             for (c1 = 0; c1 < parameters[paramName].inputDivs.length; c1 += 1) {
                 parameters[paramName].inputDivs[c1].val(paramValueNum);
             }
 
+            // Update the single slider with the new parameter's value.
             if (parameters[paramName].sliderDiv !== null) {
                 parameters[paramName].sliderDiv.slider('value', paramValueNum);
             }
 
+            // Update the helper array with the new parameter's value.
             allParameterValues[parameters[paramName].helperArrayIndex] = paramValueNum;
+
+            return true;
         } // End-of: function setParameterValue
 
         // ####################################################################
@@ -230,11 +271,8 @@ define('State', ['logme'], function (logme) {
             var paramName, newParamObj;
 
             if (typeof obj['@var'] !== 'string') {
-                logme(
-                    '[ERROR] State.processParameter(obj): obj["@var"] is not a string.',
-                    obj['@var'],
-                    '---> Not adding a parameter.'
-                );
+                logme('ERROR: Expected obj["@var"] to be a string. It is not.');
+                logme('obj["@var"] = ', obj['@var']);
 
                 return;
             }
@@ -248,14 +286,19 @@ define('State', ['logme'], function (logme) {
                 (processFloat('@step', 'step') === false) ||
                 (processFloat('@initial', 'value') === false)
             ) {
-                logme('---> Not adding a parameter named "' + paramName + '".');
+                logme('ERROR: A required property is missing. Not creating parameter "' + paramName + '"');
 
                 return;
             }
 
+            // Pointers to text input and slider DIV elements that this
+            // parameter will be attached to. Initially there are none. When we
+            // will create text inputs and sliders, we will update these
+            // properties.
             newParamObj.inputDivs = [];
             newParamObj.sliderDiv = null;
 
+            // Everything went well, so save the new parameter object.
             parameters[paramName] = newParamObj;
 
             return;
@@ -264,19 +307,16 @@ define('State', ['logme'], function (logme) {
                 var attrValue;
 
                 if (typeof obj[attrName] !== 'string') {
-                    logme(
-                        '[ERROR] state.processParameter(obj): obj["' + attrName + '"] is not a string.',
-                        obj[attrName]
-                    );
+                    logme('ERROR: Expected obj["' + attrName + '"] to be a string. It is not.');
+                    logme('obj["' + attrName + '"] = ', obj[attrName]);
 
                     return false;
                 } else {
                     attrValue = parseFloat(obj[attrName]);
 
-                    if (isNaN(attrValue) === true) {
-                        logme(
-                            '[ERROR] state.processParameter(obj): for attrName = "' + attrName + '" attrValue is NaN.'
-                        );
+                    if (isFinite(attrValue) === false) {
+                        logme('ERROR: Expected obj["' + attrName + '"] to be a valid floating-point number. It is not.');
+                        logme('obj["' + attrName + '"] = ', obj[attrName]);
 
                         return false;
                     }
@@ -288,10 +328,22 @@ define('State', ['logme'], function (logme) {
             } // End-of: function processFloat
         } // End-of: function processParameter
 
+        // ####################################################################
+        //
+        // Function: generateHelperArrays()
+        // -------------------------------
+        //
+        //
         // Populate 'allParameterNames' and 'allParameterValues' with data.
         // Link each parameter object with the corresponding helper array via
-        // an index ('helperArrayIndex'). It will be the same for both of the
+        // an index 'helperArrayIndex'. It will be the same for both of the
         // arrays.
+        //
+        // NOTE: It is important to remember to update these helper arrays
+        // whenever a new parameter is added (or one is removed), or when a
+        // parameter's value changes.
+        //
+        // ####################################################################
         function generateHelperArrays() {
             var paramName, c1;