2
\$\begingroup\$

Addressed both issues in this codereview post.

Fixed the indexing issue and the call mechanism of the function expression.

Looking for perfect code. Hope this is the last rev.

 /***************************************************************************************************
 **ALGORITHMS
 ***************************************************************************************************/
 // self used to hold client or server side global
 (function () {
 "use strict";
 // holds (Pub)lic properties
 var Pub = {},
 // holds (Priv)ate properties
 Priv = {},
 // holds "imported" library properties
 $A;
 (function manageGlobal() {
 // Priv.g holds the single global variable, used to hold all packages
 Priv.g = '$A';
 if (this[Priv.g] && this[Priv.g].pack && this[Priv.g].pack.utility) {
 this[Priv.g].pack.algo = true;
 $A = this[Priv.g];
 } else {
 throw new Error("algo requires utility module");
 }
 }());
 Pub.swap = function (arr, i, j) {
 var temp = arr[i];
 arr[i] = arr[j];
 arr[j] = temp;
 };
 // checks to see if sorted
 Pub.isSorted = function (arr) {
 var i,
 length = arr.length;
 for (i = 1; i < length; i++) {
 if (arr[i] < arr[i - 1]) {
 return false;
 }
 }
 return true;
 };
 // repeatedly orders two items ( a bubble ) at a time
 Pub.bubbleSort = function (arr) {
 var index_outer,
 index_inner,
 swapped = false,
 length = arr.length;
 for (index_outer = 0; index_outer < length; index_outer++) {
 swapped = false;
 for (index_inner = 0; index_inner < length - index_outer; index_inner++) {
 if (arr[index_inner] > arr[index_inner + 1]) {
 Pub.swap(arr, index_inner, index_inner + 1);
 swapped = true;
 }
 }
 if (!swapped) {
 break;
 }
 }
 return arr;
 };
 // repeatedly finds minimum and places it the next index
 Pub.selectionSort = function (arr) {
 var index_outer,
 index_inner,
 index_min,
 length = arr.length;
 for (index_outer = 0; index_outer < length; index_outer++) {
 index_min = index_outer;
 for (index_inner = index_outer + 1; index_inner < length; index_inner++) {
 if (arr[index_inner] < arr[index_min]) {
 index_min = index_inner;
 }
 }
 if (index_outer !== index_min) {
 Pub.swap(arr, index_outer, index_min);
 }
 }
 return arr;
 };
 // repeatedly places next item in correct spot using a "shift"
 Pub.insertionSort = function (arr) {
 var index_outer,
 index_inner,
 value,
 length = arr.length;
 for (index_outer = 1; index_outer < length; index_outer++) {
 value = arr[index_outer];
 // JavaScript optimization - index_inner >=0 is removed
 // as the array index will return undefined for a negative index
 for (index_inner = index_outer - 1; (arr[index_inner] > value);
 index_inner--) {
 arr[index_inner + 1] = arr[index_inner];
 }
 arr[index_inner + 1] = value;
 }
 return arr;
 };
 // module complete, release to outer scope
 this[Priv.g] = $A.extendSafe(this[Priv.g], Pub);
 }).call(this);
asked Jan 23, 2014 at 22:06
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

I would remove Pub.swap and copy its code where you need it.

That's because function calls are expensive, so better avoid calling a function at each iteration if possible.

And try to cache the expressions in loop's condition. For example,

var lengthMinusIndexOuter = length - index_outer;
for (index_inner = 0; index_inner < lengthMinusIndexOuter; index_inner++) {

instead of

for (index_inner = 0; index_inner < length - index_outer; index_inner++)
answered Jan 25, 2014 at 3:37
\$\endgroup\$
4
  • \$\begingroup\$ The caching sound like a good idea, but I don't think a function call is so costly that it is worth making the code less DRY. \$\endgroup\$ Commented Jan 26, 2014 at 2:15
  • \$\begingroup\$ @www.arcmarks.com You would be surprised of how costly are function calls: see slides 10~19 of Extreme JavaScript Performance slideshow \$\endgroup\$ Commented Jan 26, 2014 at 3:16
  • \$\begingroup\$ over 3 years old... \$\endgroup\$ Commented Jan 26, 2014 at 18:29
  • \$\begingroup\$ @www.arcmarks.com Well, newer javascript engines are more optimized and can do more operations per second. But inline code is still faster than function calls, because inline code runs faster too. \$\endgroup\$ Commented Jan 26, 2014 at 20:50

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.