How can i make this work with fewer lines? The goal here is to get the third biggest integer of an array.
I would also appreciate if someone polished this code.
let array = [442, 93, 3, 5, 30, 10];
const f = (arr) => {
let secureArr = [...arr];
let big0 = Math.max.apply(null, secureArr);
const index0 = secureArr.indexOf(big0);
secureArr.splice(index0, 1);
let big1 = Math.max.apply(null, secureArr);
const index1 = secureArr.indexOf(big1);
secureArr.splice(index1, 1);
let big2 = Math.max.apply(null, secureArr);
const index2 = secureArr.indexOf(big2);
secureArr.splice(index2, 1);
console.log(Math.min(big0, big1, big2));
};
f(array);
let array = [442, 93, 3, 5, 30, 10];
const f = (arr) => {
let secureArr = [...arr];
let big0 = Math.max.apply(null, secureArr);
const index0 = secureArr.indexOf(big0);
secureArr.splice(index0, 1)
let big1 = Math.max.apply(null, secureArr);
const index1 = secureArr.indexOf(big1);
secureArr.splice(index1, 1);
let big2 = Math.max.apply(null, secureArr);
const index2 = secureArr.indexOf(big2);
secureArr.splice(index2, 1);
console.log(Math.min(big0, big1, big2));
};
f(array);
4 Answers 4
- Generally, you want to separate printing from logic. Have your function return the third biggest instead of printing it, and print the return value. This allows you to include the function in a larger project without needing to make any changes.
- https://softwareengineering.stackexchange.com/q/364086 and https://stackoverflow.com/q/34361379/2336725 recommend
function f(arr) {}
instead ofconst f = (arr) => {}
. Butf
is a terrible name for a function. Let's change it tothirdBiggest
. - There's no real point to the
Math.min
at the end. You know thatbig0
is the biggest,big1
is the biggest after that, andbig2
the biggest after that. Lets just returnbig2
. - You never change the
big#
values, so they should be constants. But it would be better to reuse thebig#
andindex#
variables. I'm going to rename them tobig
andbigIndex
. - But now we see that we're repeating the exact same steps three times. Let's move that into a for loop.
- We might as well use an array spread instead of
.apply
secureArr
implies some sort of security consideration, which I'm not seeing. If you're just wanting to not modify the original, lets call it a copy. Here's where we are now:
let array = [442, 93, 3, 5, 30, 10];
function thirdBiggest(arr) {
let arrCopy = [...arr];
let big, bigIndex;
for ( let i = 0 ; i < 3 ; i++ ) {
big = Math.max(...arrCopy);
bigIndex = arrCopy.indexOf(big);
arrCopy.splice(bigIndex, 1);
}
return big;
}
console.log(thirdBiggest(array));
- You specifically mentioned fewer lines. This is not usually a good metric. If that's all you want, then you should go with Bens Steves' approach and your function will be two lines long. But it will be measurably slower for large arrays.
- "third biggest" is a bit arbitrary, and now that we have a loop, we can see that it's not hard to turn that into a parameter. Let's do that, and change
thirdBiggest
intonthBiggest
. - The usual approach for this problem is that you go through the array once, and keep track of the highest values you've seen so far. If you see a new highest value, it pushes the bottom one off of the list. That ends up being:
function nthBiggest(arr,n) {
// start with the front of the array
let biggestValues = arr.slice(0,n);
biggestValues.sort( (a,b) => a-b );
// now look through the rest of the array
arr.slice(n).forEach( (value) => {
if ( value > biggestValues[0] ) {
biggestValues.shift();
let valueIndex = biggestValues.findIndex( (big) => big>value );
if ( valueIndex >= 0 ) {
biggestValues.splice(valueIndex,0,value);
} else {
biggestValues.push(value);
}
}
});
return biggestValues[0];
}
console.log(nthBiggest(array,3));
That's more lines than your original version, but should be the fastest.
-
2\$\begingroup\$ The use of an array for
biggestValues
is problematic from an algorithmic complexity point of view. The issue is that all operations on the array are O(n), and therefore usingnthBiggest
to find the minimum will have O(n*n) complexity. The most efficient there would be a min-heap, I expect, to get that down to O(n log n). And... that's when you realize that this algorithm has a name: this is the Selection Algorithm, and there's quite a bit of literature about it. \$\endgroup\$Matthieu M.– Matthieu M.2022年01月27日 10:39:19 +00:00Commented Jan 27, 2022 at 10:39 -
\$\begingroup\$ @MatthieuM.: The size of the
biggestValues
is different from the size of the wholearr
. Let's sayn
(n biggest) andm
(arr.length). Then yes, the worst case isO(n*m)
in the case where arr has many new max-candidates. (e.g. an overall increasing trend, so the number of updates to the candidate list is also O(m)). But the average case for random data is lower, isn't it? The likelihood of seeing a value higher than any of the highest 5 you've seen before decreases the more you see, I'd guess quickly enough to matter. Unless you use a branchless SIMD sorting network to update it.. \$\endgroup\$Peter Cordes– Peter Cordes2022年01月28日 02:00:02 +00:00Commented Jan 28, 2022 at 2:00 -
\$\begingroup\$ Ah, in comments on the sorting answer, you already described the question's approach as
O(n*k)
. Oh, and you said using this to find the minimum, so calling it withn = m
. That would basically be user error; this version is clearly optimized forn << m
, and if you want the min of a large array you should do that. \$\endgroup\$Peter Cordes– Peter Cordes2022年01月28日 02:01:43 +00:00Commented Jan 28, 2022 at 2:01 -
\$\begingroup\$ It's also insane to only return one value, when in fact you're finding all
n
largest values: just return the sorted list so finding the 3 largest values takes one call to this function, one pass over the data. That's the whole point of this vs. max / search / replace with a tiny value or remove / repeat, for arrays that fit in cache. (Plain Max and indexof are optimizable with SIMD, and in Python hopefully at least have compiled C implementations even if it's just scalar. \$\endgroup\$Peter Cordes– Peter Cordes2022年01月28日 02:08:25 +00:00Commented Jan 28, 2022 at 2:08
Generally
To get the n
biggest integer in an array you want to first sort the array of integers. Finally, you can index the array by array.length - n
.
Example: Third Largest Number in Array
const arrNumbers = [442, 93, 3, 5, 30, 10];
// First Step: Sort Array
const sortedArray = arrNumbers.sort((a,b) => a - b);
// Final Step: Get Third to Last Number in Array
const thirdLargest = sortedArray[sortedArray.length - 3];
console.log(thirdLargest);
const assertion = thirdLargest === 30;
console.log(assertion)
-
2\$\begingroup\$ But sorting is an
n log n
operation. OP's approach isO(n)
. \$\endgroup\$Teepeemm– Teepeemm2022年01月27日 02:51:16 +00:00Commented Jan 27, 2022 at 2:51 -
3\$\begingroup\$ @Teepeemm: For a generic version of selecting the k-th largest, OP's approach is O(n * k); depending how large
k
is related tolog n
, sorting may offer better complexity. Also... this approach has the advantage of being very easy to understand, which is a clear advantage. \$\endgroup\$Matthieu M.– Matthieu M.2022年01月27日 10:41:48 +00:00Commented Jan 27, 2022 at 10:41 -
\$\begingroup\$ For top k numbers, you can maintain a k-size max-heap. But that is probably not faster in practice for small k. \$\endgroup\$qwr– qwr2022年01月27日 18:12:29 +00:00Commented Jan 27, 2022 at 18:12
- Naming could be better
- I prefer
list
overarray
, shorter to type and avoids the temptation to usearr
as a variable f
is a meaningless name for a function, go with<verb><thing>
- I prefer
let big0
could beconst big0
- you use
index0
only once, I would have usedsecureArr.indexOf(big0)
in thesplice
call - The code has a block of code that got copy pasted twice, consider applying DRY techniques
- Personally, I would replace this with a one-liner until I see it really is too slow
- If it did get too slow, I would replace this with an old fashioned
for
loop let big1 = Math.max.apply(null, secureArr);
->let big1 = Math.max(...secureArr);
- Calculating and outputting should happen in different locations
- I think using the
function
keyword would have been more readable
Mandatory rewrite
const input = [442, 93, 3, 5, 30, 10];
function getNthLargest(list, n=3){
return [...list.sort((a, b)=>b-a)][n-1];
}
console.log(getNthLargest(input,3));
In general, to find the kth largest number, calling indexOf
k times, with each call linear time, will lead to O(n*k) time complexity. (Also splice()
may be slow - not sure if this is linear time per call). This is fine for small k.
A one-liner solution is to sort an array and take the kth largest number. This is O(n log n) and I assume the simplest and fastest method in practice, since sorting integers should be optimized.
For k much smaller than n, you can iterate over all values and track the top-k smallest values in a min-heap limited to size k (inserting new value, then popping the smallest value at the top), then at the end you are left with the top-k largest values, with kth largest at the top of the heap, in total O(n log k) time. This generalizes maintaining the max value for the k=1 case. You can also maintain order statistics with an order-statistic tree if you need faster lookup, insertion, and deletion. This is overkill for a simple array.
-
\$\begingroup\$ You'd want a min-heap since you want to remove the smallest out of the k-highest every time you insert a new candidate that's higher than the lowest of the candidate's you've seen thus far. But yes, then you can return that whole list of k'th highest values, instead of just one of them, so the caller only needs to call it once, not once for each output value. \$\endgroup\$Peter Cordes– Peter Cordes2022年01月28日 02:09:44 +00:00Commented Jan 28, 2022 at 2:09
-
\$\begingroup\$ @PeterCordes yeah you're right. It's been a while since I implemented this but afterwards, the kth largest value should be at the top of the heap and can be popped and returned. \$\endgroup\$qwr– qwr2022年01月28日 03:59:55 +00:00Commented Jan 28, 2022 at 3:59