HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

Implementing RadixSort using JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptradixsortimplementingusing

Problem

Can someone review my code? I am trying to implement radix sort in JavaScript.



`var numbers = [90, 46, 7, 12, 100, 68];
var length = numbers.length;
var buckets = new Array(10);

Clear();
function Clear() {
for (var i = 0; i 0) {
number = parseInt(number / 10);
count++;
}
return count;
}

function CopyToArray() {
var k = 0;
for (var i = 0; i

Solution

Your code is not finished.

I could not say this is a finished bit of code as it simply does not work. A simple test, a random set of integers and comparing it to the result of the standard sort showed that more often than not the function just failed.

Read this Gramma and types

3 rules of coding:

  • There is no code without testing.



  • There is no code without testing.



  • There is no code without testing.



When creating a function such as this it is important that you have a reliable means of testing the code over all possible inputs. One test case just does not cut it. Having a full test at the start makes writing the code a lot easier. You can make a change and instantly test all possible inputs, it also helps you optimise the code once you have the basics worked out.

Example test function

This is the function that I used to test your code. It creates a random list of numbers of different lengths, different numbers of digits, and different ranges of digits. If it fails any test then it stops displaying the failed array and the correct array.

function test() {
    function log (data) {console.log(data); }
    function createRandomArray (maxItems, maxDigits, digitRange) {
        var numSize, num;
        var numbers = [];
        var count = Math.random() * maxItems;
        while (count-- > 0) {
            numSize = Math.random() * maxDigits;
            num = 0;
            while (numSize-- > 0) {
                num *= 10;
                num += Math.floor(Math.random() * digitRange);
            }
            numbers.push(num);
        }
        return numbers;
    }
    var items, digits, range, array;
    for (items = 0; items  a - b);
                if (array.join(",") !== numbers.join(",")) {
                    log("==============================");
                    log("Test failed.");
                    log("Items = "+length)
                    log("Max digits = "+digits)
                    log("Digit range = "+range)
                    log("- Result --------------------")
                    numbers.forEach(n => log(n))
                    log("- Expected ------------------")
                    array.forEach(n => log(n) )
                    return
                }
            }
        }
    }
    log("All passed")
}
test();


Style and syntax.

-
Only capitalise variables if they are functions that are intended to be instantiated via the new token. Note the syntax highlighting.

function MyObj(){}   // good
var obj = new MyObj();

function myObj(){}   // bad
var obj = new myObj();

function doSomething(){} // good

function DoSomething(){} // bad

var myData = []; // good

var MyData = []; // bad.


-
Don't take shortcuts when it comes to syntax, especial concerning curly braces when defining blocks, it is a major source of syntax related bugs that are very hard to find when you are staring at thousands of lines of code.

for(i = 0; i < 10; i+= 1) 
    doSomethif();            // bad

for(i = 0; i < 10; i+= 1) doSomethif();  // better but still bad

for(i = 0; i < 10; i+= 1) {  doSomethif(); }  // OK

for(i = 0; i < 10; i+= 1) {    // Good
    doSomethif(); 
}


Same for if, while, do statement blocks.

-
Layout. Don't spread out initialization code throughout the source. You are making calls to clear and radixSort intermixed within the source. Ok maybe for a short bit of code but when your code gets over a few pages it can be very hard to see what is going on.

The general layout is Function declarations first, then variables, and then functional source. I personally use bottom up layout style. The deepest level function first, top of the page, then down the page to the highest level function. So in most cases you know that to locate a function being called from within another is to look above the current functions location.

You can also use top down, it does not matter which direction, just as long as you are consistent.

-
Naming. Use clear descriptive words or phrases for the functions. You have Clear. Clear what, maybe resetBuckets would be a better name for the function.

-
Use ES6. There is no reason not to use Javascript latest and best incarnation. Using ES5 because it has more browsers that support it is only shooting your self in the foot. Time moves along and ES6 will soon be ES7. As a programmer you can not afford to not learn the latest and best, by the time you know it back to front, it will be obsolete.

-
Use const for any variables the will not change. (not a must, but it makes you look more professional) The general accepted reasoning between the use of constants const, block scope let, and function scope var variables is mostly about reducing bugs by protecting state. You cant mess up the state of the code if it is a protected 'const' or inaccessible (out of scope), which makes sense, well kind of, if you feel that you need the training wheels down that is.

Understand what the differences are and use the

Code Snippets

function test() {
    function log (data) {console.log(data); }
    function createRandomArray (maxItems, maxDigits, digitRange) {
        var numSize, num;
        var numbers = [];
        var count = Math.random() * maxItems;
        while (count-- > 0) {
            numSize = Math.random() * maxDigits;
            num = 0;
            while (numSize-- > 0) {
                num *= 10;
                num += Math.floor(Math.random() * digitRange);
            }
            numbers.push(num);
        }
        return numbers;
    }
    var items, digits, range, array;
    for (items = 0; items <= 20; items += 1) {
        for (digits = 1; digits <= 9; digits += 1) {
            for (range = 1; range <= 9; range += 1) {
                array = createRandomArray(items, digits, range);
                length = array.length;
                numbers = [...array];
                RadixSort();
                array.sort((a, b) => a - b);
                if (array.join(",") !== numbers.join(",")) {
                    log("==============================");
                    log("Test failed.");
                    log("Items = "+length)
                    log("Max digits = "+digits)
                    log("Digit range = "+range)
                    log("- Result --------------------")
                    numbers.forEach(n => log(n))
                    log("- Expected ------------------")
                    array.forEach(n => log(n) )
                    return
                }
            }
        }
    }
    log("All passed")
}
test();
function MyObj(){}   // good
var obj = new MyObj();

function myObj(){}   // bad
var obj = new myObj();

function doSomething(){} // good

function DoSomething(){} // bad

var myData = []; // good

var MyData = []; // bad.
for(i = 0; i < 10; i+= 1) 
    doSomethif();            // bad

for(i = 0; i < 10; i+= 1) doSomethif();  // better but still bad

for(i = 0; i < 10; i+= 1) {  doSomethif(); }  // OK

for(i = 0; i < 10; i+= 1) {    // Good
    doSomethif(); 
}
// pass to the function data
function radixSort(numArray){
    // define required functions within the function
    function Clear() {
        for (var i = 0; i < 10; i++)
            buckets[i] = [];
    }
    // declare inside the function
    var length = numArray.length;
    var buckets = new Array(10);

    // return the processed information
    return sorted

    // Or leave in place but still return the array
    return numArray
 }
// bad
function clear() {
    for (var i = 0; i < 10; i++) {
        buckets[i] = []; // dereferences the array held in [i] and creates a brand new one
    }
}

// good
function clear() {
    for (var i = 0; i < 10; i++) {
        buckets[i].length = 0; // reduces the array size but does not create a new one
    }
}

// great by hard work
// first array item is the current number of usable items in the array
function clear() {
    for (var i = 0; i < 11; i++) {
        buckets[i][0] = 0; // totally clean nothing created, nothing deleted, and very fast
                       // but you can no longer use bucket[i].length.
    }
}

Context

StackExchange Code Review Q#144803, answer score: 4

Revisions (0)

No revisions yet.