4
\$\begingroup\$

I have script for get three values (minimal numbers) from array :

$(document).ready(function(){
    var _array = [1,2,3,4];
    var save = 0;
    var _new_array = [];
    var nearDistance = [];
    $.each(new Array(3),function(n){
        if (_new_array.length < 1){
            save = Math.min.apply(Math,_array);
            _new_array = jQuery.grep(_array, function(value) {
              return value != save;
            });
            nearDistance.push(save);
        } else {
            save = Math.min.apply(Math,_new_array);
            _new_array = jQuery.grep(_new_array, function(value) {
              return value != save;
            });
            nearDistance.push(save);
        }
    });
    console.log(nearDistance);    
});

This script works to get 3 minimal values,

scenario script : Do each 3 times, first each get minimal value (save) from first array (_array), delete minimal value from the first array (_array) and then put resudial values to new array (_new_array), minimal value push to nearDistance, next each get minimal value from new array.

I think my script too less performance if I want to get more values from an array, what do you think about my script?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Please don't invalidate answers by editing the code. \$\endgroup\$ Sep 19, 2015 at 17:16
  • \$\begingroup\$ @EthanBierlein sorry my bad, I didn't edit the code but simplify the code from joseph suggestion. \$\endgroup\$
    – itx
    Sep 19, 2015 at 17:22

3 Answers 3

6
\$\begingroup\$

If I understand your question correctly, you're trying to:

  • Get n smallest numbers from your array.
  • The original array doesn't get mutated.

First off:

  • Underscores have a special meaning in programming. They're used to denote pseudo-private variables. Since JS has no concept of private (unless emulated with closures), prefixing an underscore to a publicly accessible variable means not to access that variable directly.

  • For this operation, you don't need jQuery. Vanilla JS has it's own forEach for $.each and filter for $.grep.

  • Just so you know, $ and jQuery are the same. The $ is an alias made for convenience. jQuery is the global of jQuery to prevent issue with clobbering. IIRC, MooTools uses the $ global. Anyone can alias their library as $, but who names their library "jQuery" besides jQuery?

  • $(document).ready(fn) can be shortened to $(fn) or jQuery(fn).

  • This operation is best moved out to a function so that you can call this out, pass in an array, and get the results you want.

  • Another problem is that $.grep return a new array every call. That means on each iteration of $.each, you're creating a new array - which is bad.

  • Math.min in your code will always return the smallest number. This means on every push, the numbers run from smallest to largest. Why not sort the array first and slice the first n?

Running with the above assumptions and review, you can do this:

function getSmallestNumbers(array, n) {
  // We slice first to prevent mutation of the original array when we
  // do `sort`. Then we slice again to get n items.
  return array.slice().sort().slice(0, n);
}

var distances = [4, 3, 1, 2];
var nearDistances = getSmallestNumbers(distances, 3);

// SE should seriously have a console view
document.write(JSON.stringify(nearDistances));

\$\endgroup\$
3
  • \$\begingroup\$ Good catch about the 'original array doesn't get mutated'. Slice fixes this :) \$\endgroup\$
    – Pinoniq
    Sep 18, 2015 at 13:29
  • \$\begingroup\$ Nice!, thanks @joseph for some suggestion about syntax, I have try your solution but why still get wrong number If an array have some number like [22.4, 1.1, 102.14, 89.5], I think that sort every character of value not per value, sound like sort with ascending A to Z \$\endgroup\$
    – itx
    Sep 19, 2015 at 16:44
  • \$\begingroup\$ @itx You can provide a sort with a custom sorting function. indofraiser's answer has that detail. \$\endgroup\$
    – Joseph
    Sep 19, 2015 at 18:45
1
\$\begingroup\$

what do you think about my script?

You really want to know what I think? Cause it aint pretty...

I don't mind reading bad english, because not everyone is a native speaker. But that's not the problem here. nearDistance, _newArray, save, ... are just really bad variable names. They tell me nothing.

I mean, whats it with that underscore? Where are the comments explaining what and why? Why are you doing a $.each(new Array()) instead of a simple for loop? Why are you defining variables outside of the scope they are used in?

I think you ran into a couple of meta-problems here. So let's take a step back!

Work smarter not harder

I'm still not sure what exactly your code should do, but doesn't this also work?

var myArray = [1,2,3,4];
var newArray = myArray.sort().slice(0,3);
\$\endgroup\$
0
0
\$\begingroup\$

First comment echos @Pinoniq tidy up your variable names. Instantly it will look cleaner.

Second, you are repeating the same save code, so move it into a later block. Better yet, class it from a separate class.

You should also determine if you need to have so much code for firstArray when you do not need to change it.

I've done this in Notepad so it's not checked:

$(document).ready(function(){
    var firstArray = [1,2,3,4];
    var answer = 0;
    var newArray = [];
    var nearDistance = [];

    $.each(newArray (3),function(n){
        if (newArray .length < 1){
            answer = Math.min.apply(firstArray );
            newArray  = jQuery.grep(firstArray , function(value) {
        } else {
            answer = Math.min.apply(Math,newArray ); 
            newArray  = jQuery.grep(newArray  , function(value) {

        }    
               return value != answer
            });
            nearDistance.push(answer);
    });
    console.log(nearDistance);    
});

You should really look at:

var _array = [1,3,2];
Math.max.apply(Math,_array); // 3
Math.min.apply(Math,_array); // 1

on this feed and I would strongly suggest looking at this link.

An example:

 function sortfunction(a, b){
     return (a - b) //causes an array to be sorted numerically and ascending
     }
\$\endgroup\$

Your Answer

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

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