7
\$\begingroup\$

I am concerned where it can be improved.

  • Main concerns are coding style.
  • Any other improvements are welcome too.
  • Priority is on JavaScript, review for HTML/CSS are welcome too.

What it does:

  • Simulate Bubble Sort graphically with animations.
  • Simulate Selection Sort graphically with animations.
  • There are some visual kinks in Selection Sort simulation but it sorts correctly and works to the best of my knowledge.

Information:

  • Contains Single HTML Page,Single CSS file and Single JavaScript file
  • Prototype.js => v1.7
  • script.aculo.us => v1.9.0
  • Screen clipping, just a part of it

enter image description here


JavaScript: base.js

/**
 * Bhathiya Perera IT110057-04
 *
 */

/**
 * Draws an array with indexes in box structure
 * @param {Object} arr Array to be drawn
 */
function drawarray(arr) {
    var boxset = $("boxset");
    var boxsetnum = $("boxsetnum");
    boxset.innerHTML = "";
    boxsetnum.innerHTML = "";

    boxset.hide();
    boxsetnum.hide();

    var boxsetStr = "";
    var boxsetnumStr = "";

    for (var i = 0; i < arr.length; i++) {
        boxsetStr = boxsetStr + "<div class=box id='arrbox" + i.toString() + "' >" + arr[i] + "<\/div>"
        boxsetnumStr = boxsetnumStr + "<div class=box2>" + (i + 1) + "<\/div>"
    }

    boxset.innerHTML = boxsetStr;
    boxsetnum.innerHTML = boxsetnumStr;

    boxset.show();
    boxsetnum.show();

}

/**
 * Simulate Bubble Sort (Ascending only)
 * @param {Object} a Array (Must be an integer array)
 */
function bubbleSort(a) {
    var swapped;
    do {
        swapped = false;
        for (var i = 0; i < a.length - 1; i++) {
            if (a[i] > a[i + 1]) {
                new Effect.Move("arrbox" + i.toString(), {
                    x : 32,
                    y : 0,
                    queue : 'end'
                });
                new Effect.Move("arrbox" + (i + 1).toString(), {
                    x : -32,
                    y : 0,
                    queue : 'end'
                });
                new Effect.Move("arrbox" + i.toString(), {
                    x : 0,
                    y : 32,
                    queue : 'end'
                });
                new Effect.Move("arrbox" + (i + 1).toString(), {
                    x : 0,
                    y : -32,
                    queue : 'end'
                });
                new Effect.Move("arrbox" + i.toString(), {
                    x : -32,
                    y : 0,
                    queue : 'end'
                });
                new Effect.Move("arrbox" + (i + 1).toString(), {
                    x : 32,
                    y : 0,
                    queue : 'end'
                });
                var temp = a[i];
                var t1 = document.getElementById("arrbox" + i.toString());
                var t2 = document.getElementById("arrbox" + (i + 1).toString());
                var t3 = t1.id;
                t1.id = t2.id;
                t2.id = t3;
                a[i] = a[i + 1];
                a[i + 1] = temp;
                swapped = true;
            }
        }
    } while (swapped);
}

/**
 * This function must be called by the simulate button
 * And this will initialize the array and pass it to  bubbleSort()
 */
function bubble_simulate() {
    var numbers = $("arrayinput").value;
    var toBeSorted = numbers.split(",");
    if (toBeSorted.length >= 1) {
        var toBeSortedi = new Array();
        for (var q = 0; q < toBeSorted.length; q++) {
            toBeSortedi[q] = parseInt(toBeSorted[q]);
        }
        drawarray(toBeSortedi);
        bubbleSort(toBeSortedi);
    }
}
/**
 * Simulate Selection Sort 
 * @param {Object} sortMe array to be sorted
 */
function selectionSort(sortMe) {
    var i, j, small, tmp2;
    //small contains the smallest value
    //tmp2 is used only for swapping
    for ( i = 0; i < sortMe.length; i++) {
        small = i;

        new Effect.Move("arrbox" + small.toString(), {
                    x : 32,
                    y : 0,
                    queue : 'end'
                });
        for ( j = i + 1; j < sortMe.length; j++) {
            if (sortMe[j] < sortMe[small]) {
                //drop the current smallest (its j)
                new Effect.Move("arrbox" + small.toString(), {
                    x : -32,
                    y : 0,
                    queue : 'end'
                });
                small = j;
                //raise new smallest (its j)
                new Effect.Move("arrbox" + j.toString(), {
                    x : 32,
                    y : 0,
                    queue : 'end'
                });
            }
        }
        //swapping 
        var l = (small-i)*32; //length difference between the two array elements
        new Effect.Move("arrbox" + i.toString(), {
            x : -32,
            y : 0,
            queue : 'end'
        }); 
        new Effect.Move("arrbox" + i.toString(), {
            x : 0,
            y : l,
            queue : 'end'
        });
        new Effect.Move("arrbox" + small.toString(), {
            x : 0,
            y : -l,
            queue : 'end'
        });
        new Effect.Move("arrbox" + small.toString(), {
            x : -32,
            y : 0,
            queue : 'end'
        });     
        new Effect.Move("arrbox" + i.toString(), {
            x : 32,
            y : 0,
            queue : 'end'
        });         
        tmp2 = sortMe[small];
        sortMe[small] = sortMe[i];
        sortMe[i] = tmp2;
        //swap id-s'
        var t1 = document.getElementById("arrbox" + i.toString());
        var t2 = document.getElementById("arrbox" + small.toString());
        var t3 = t1.id;
        t1.id = t2.id;
        t2.id = t3;
    }
}
/**
* Simulate seelction Sort 
* The simulate button will call this
*/
function selection_simulate() {
    var numbers = $("arrayinput").value;
    var toBeSorted = numbers.split(",");
    if (toBeSorted.length >= 1) {
        var toBeSortedi = new Array();
        for (var q = 0; q < toBeSorted.length; q++) {
            toBeSortedi[q] = parseInt(toBeSorted[q]);
        }
        drawarray(toBeSortedi);
        selectionSort(toBeSortedi);
    }
}
function show_selection_sort() {
    var strVar="";
    strVar += "<h2>Selection sort<\/h2>";
    strVar += "A{";
    strVar += "<input type=\"text\" id=\"arrayinput\" style=\"width:550px;\"\/>";
    strVar += "}";
    strVar += "<hr style=\"visibility:hidden;\"\/>";
    strVar += "<div class=\"dbutton\" onclick=\"selection_simulate();\">";
    strVar += "Simulate";
    strVar += "<\/div>";
    strVar += "<hr style=\"visibility:hidden;\"\/>";
    strVar += "<table>";
    strVar += "<tr>";
    strVar += "<td><div id=\"boxsetnum\">";
    strVar += "<\/div><\/td>";
    strVar += "<td><div id=\"boxset\">";
    strVar += "<\/div><\/td>";
    strVar += "<\/tr>";
    strVar += "<\/table> ";
    var simulbox = $("simulbox");
    simulbox.innerHTML = strVar;
    simulbox.highlight({
        startcolor : '#D0E0D0',
        endcolor : '#DDDDDD'
    });

}
function show_bubble_sort() {
    var strVar="";
    strVar += "<h2>Bubble sort<\/h2>";
    strVar += "A{";
    strVar += "<input type=\"text\" id=\"arrayinput\" style=\"width:550px;\"\/>";
    strVar += "}";
    strVar += "<hr style=\"visibility:hidden;\"\/>";
    strVar += "<div class=\"dbutton\" onclick=\"bubble_simulate();\">";
    strVar += "Simulate";
    strVar += "<\/div>";
    strVar += "<hr style=\"visibility:hidden;\"\/>";
    strVar += "<table>";
    strVar += "<tr>";
    strVar += "<td><div id=\"boxsetnum\">";
    strVar += "<\/div><\/td>";
    strVar += "<td><div id=\"boxset\">";
    strVar += "<\/div><\/td>";
    strVar += "<\/tr>";
    strVar += "<\/table> ";

    var simulbox = $("simulbox");
    simulbox.innerHTML = strVar;
    simulbox.highlight({
        startcolor : '#D0E0D0',
        endcolor : '#DDDDDD'
    });

}

HTML: index.html

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
"http://www.w3.org/TR/html4/loose.dtd">
<html>
    <head>
        <meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
        <script src="javascripts/lib/prototype.js" type="text/javascript"></script>
        <script src="javascripts/lib/scriptaculous.js?load=effects" type="text/javascript"></script>
        <script src="javascripts/algo/base.js" type="text/javascript">

        </script>
        <link rel="stylesheet" type="text/css" href="style/style.css" />
        <title>DAA Algorithm Simulator IT110057-04</title>
    </head>
    <body>
        <div id="maindiv">
            <h1>Algorithem Simulator</h1>
            <p style="text-align:center;">
                This web page was created by Bhathiya Perera for DAA Assignment.
                <br />
                IT110057-04 Perera B.A.B.N.H.
            </p>
            <hr>
            <table style="text-align:center;">
                <tr id="algoselect">
                    <td>
                    <div class="algobox"  onclick="show_bubble_sort();" style="float:left;">
                        <h2>Bubble Sort</h2>
                        <p>
                            Bubble Sort is a simple sorting algorithm that works by repeatedly stepping through the list to be sorted
                        </p>
                    </div></td>
                    <td>
                    <div class="algobox"  onclick="show_selection_sort();" style="float:right;">
                        <h2>Selection Sort</h2>
                        <p>
                            Selection sort is simple a sorting algorithm, specifically an in-place comparison sort.
                        </p>
                    </div></td>
                </tr>
                <tr>
                    <td colspan="3">
                    <div class="simulbox" id="simulbox">
                        <h2>Welcome to Alogorithm Simulator</h2>
                        <p>Please select an algorithm to continue.</p>
                    </div></td>
                </tr>
            </table>

        </div>
    </body>

</html>

CSS: style.css

body {
    background-image: url('./light_bg.png');
    background-repeat: repeat;
}
h1 {
    background-color: #EEEEEE;
    text-align: center;
}
div#maindiv {
    margin: 0 auto;
    width: 670px;
    height: 1200px;
    background-color: #eeeeee;
    font-family: Monospace;
    padding-left: 20px;
    padding-right: 20px;
    border: #D0C0D0 1px solid;
    -moz-border-radius: 5px;
    -webkit-border-radius: 5px;
    border-radius: 5px;
    -moz-box-shadow: 3px 3px 4px #000;
    -webkit-box-shadow: 3px 3px 4px #000;
    box-shadow: 3px 3px 4px #000;
    /* For IE 8 */
    -ms-filter: "progid:DXImageTransform.Microsoft.Shadow(Strength=4, Direction=135, Color='#000000')";
    /* For IE 5.5 - 7 */
    filter: progid:DXImageTransform.Microsoft.Shadow(Strength=4, Direction=135, Color='#000000');
}
div.box {
    width: 30px;
    height: 30px;
    text-align: center;
    background-color: #ffffff;
    border: #000000 1px solid;
}
div.box2 {
    width: 32px;
    height: 22px;
    text-align: center;
    margin-right: 35px;
    padding-top: 10px;
}
div.simulbox {
    text-align: left;
    padding: 30px;
    background-color: #DDDDDD;
    width: 600px;
    height: 600px;
    -moz-border-radius: 20px;
    -webkit-border-radius: 20px;
    border-radius: 20px;
    border: #D0C0D0 1px solid;
}
div.algobox {
    cursor: pointer;
    cursor: hand;
    background-color: #DDDDDD;
    width: 215px;
    height: 220px;
    text-align: center;
    -moz-border-radius: 20px;
    -webkit-border-radius: 20px;
    border-radius: 20px;
    border: #D0C0D0 1px solid;
    margin-bottom: 10px;
    margin-top: 10px;
}
div.algobox:hover {
    background-color: #E0D0E0;
}
div.algobox:active {
    background-color: #D0C0D0;
}

div.dbutton {
    cursor: pointer;
    cursor: hand;
    background-color: #FFFFFF;
    margin-left: 5px;
    margin-right: 5px;
    width: 200px;
    height: 20px;
    text-align: center;
    -moz-border-radius: 5px;
    -webkit-border-radius: 5px;
    border-radius: 5px;
    border: #D0C0D0 1px solid;
}
div.dbutton:hover {
    background-color: #E0E0F0;
}
div.dbutton:active {
    background-color: #D0D0E0;
}

Related Links:

\$\endgroup\$
  • \$\begingroup\$ You can ignore the strike through part, It works as intended. (Although you might find that it animates weirdly,It's not a bug,It's exactly how the sort happens) \$\endgroup\$ – bhathiya-perera Aug 24 '14 at 5:47
4
\$\begingroup\$

Disclaimer: I am familiar with neither Prototype nor Scriptaculous.

JS

  • drawarray() should be named drawArray().

  • You can take advantage of += instead of boxsetStr = boxsetStr + ....

  • / is not a special character in strings and does not need to be escaped. "<\/div>" is clutter; it's harder to read than "</div>".

  • You have a ton of repetition. In particular, your parameters for Effect.Move are repeated. You should store them in a constant.

  • When concatenating a variable to a string, it is implicitly converted. You don't need to explicitly call i.toString(). Combining this with the previous point, we get:

    var MOVE_LEFT = { x: -32, y: 0, queue: "end" };
    ...
    new Effect.move("arrbox" + i, MOVE_LEFT);
    
  • Continuing from the above, it would make more sense to put the swapping animation in a separate callable function.

  • In bubble_simulate(), you use parseInt without explicitly stating the base. In general, you should explicitly state the base with parseInt(toBeSorted[q], 10). This is because in older browsers which do not comply with ECMAScript 5, strings leading with a 0 are treated as octal numbers, so parseInt("010") => 8. In practice, this is not the current behaviour of any of the major browsers.

  • In selectionSort, you declare your loop variables at the top, while in bubbleSort, you declared i in your loop. This would be fine if you actually reused your loop variables in selectionSort, but you don't; the resulting inconsistency strikes me as odd.

  • In show_selection_sort(), it seems like you would benefit greatly from templating. It certainly looks extremely busy.

HTML

  • You used <table> for layout. No. Bad. <table>s are for data, not for layout. (Exception: HTML for email.) You should be using <div>s.

  • There's some unnecessary whitespace where you included algo/base.js. There's some more after </table> and </body>. I say unnecessary because these do not make your structure any easier to read.

  • For some reason you have style attributes on things where some improved CSS could easily apply.

CSS

  • You're using cursor: pointer; cursor: hand;. This is no longer necessary; the last browser to support cursor: hand but not cursor: pointer was IE 5.5, which was dropped from support almost ten years ago.

  • Vendor prefixes for border-radius have not been required for quite a while. This article on css-tricks.com estimates less than 1% of 2012 traffic to support prefixed versions but not the bare version.

  • In general, people like their CSS to be ordered in some fashion, whether grouped by category or alphabetically. I can't seem to detect an ordering for your CSS. (Of all the points made, this one matters the least.)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.