Your function is really easy to read and follow.
I, personally, hate the double-space indentation, but that's a preference.
But there are a few things that you can improve:
- You have absolutely no input validation.
Verifying that you have more than 2 arguments is enough, and should be accomplished before running Array.prototype.slice.call(arguments);
.
Array.prototype.slice
is very slow for long arrays and array-like objects.
The array-like object arguments
has a length
property, refering to how many arguments were provided.
More ahead, you don't check if you are comparing numbers with strings, arrays, functions, regular expressions and what-not, which may lead to weird results.
- Your
for
loop is not optimal:
- You are starting from index 0.
If you store the previous number into a local variable, you can start your loop at 1. This is 1 less iteration, 1 less check and 1 less increment.
- You keep reading the
numArr.length
.
Consider storing this value in a local variable.
Many will say that it doesn't matter much, and that this won't make much of a difference. Which is true, for smaller arrays.
- You are currently building a truthy table, and checking if there's any
false
in it.
It is a lot easier if you simply return false;
as soon as one of the conditions doesn't return true
.
This avoids forcing you to check every single item ahead needlessly, since you knew the result before finishing the loop.
Aditionally, in contrast to what @hoffmale suggests, do not use if (numArr[num] >= numArr[num + 1] || isNaN(numArr[num]) || isNaN(numArr[num + 1]))
.
This is why I say it:
- The comparisson is made first, before typechecking
- The current number is checked
- The next number is checked (on the next iteration, the "current number" will be the "next number").
This makes it so that you are checking the same number twice.
Doing the typechecking before comparing, and doing only to the current number will reduce the required checks by a large amount.
As such, you can write your function like this (keeping your style intact):
function isIncreasingSequence() {
/**Check if numbers sequence is increasing
* @param {number} numbers - a sequence of input numbers;
* @return {boolean} - true if given sequence is increasing, false othrewise
*/
var length = arguments.length;
if(length < 2)
{
throw new TypeError('At least 2 arguments are required');
}
var numbers = Array.prototype.slice.call(arguments);
var old = numbers[0];
if(isNaN(old))
{
throw new TypeError('Invalid number on position 0');
}
for (var i = 1; i < length; i++)
{
if(isNaN(numbers[i] / 1))
{
throw new TypeError('Invalid number on position ' + i);
}
if(old > numbers[i])
{
return false;
}
old = numbers[i];
}
return true;
}
But still, the code isn't optimal.
Running the following code:
console.time('code');
for(var i = 1e4; i--; )isIncreasingSequence(0, 0.2, 0.3, 1);
console.timeEnd('code');
Shows that it still takes 4-5 milliseconds, on Google Chrome (on my pc), while @Kruga's ES6 answer is a lot faster (0.9-2 milliseconds).
Here are some improvements I've made, without any regards to keep the programming style integrity:
- Got rid of
Array.prototype.slice.call(arguments);
and access the arguments
object directly.
- Store the current number in a variable, reducing one more iteration and multiple accesses to the array, speeding up the code a lot.
- Start the loop at 2, instead of 0 or 1.
Here's how I've made it into:
function isIncreasingSequence() {
/**Check if numbers sequence is increasing
* @param {number} numbers - a sequence of input numbers;
* @return {boolean} - true if given sequence is increasing, false othrewise
*/
var length = arguments.length;
if(length < 2)
{
throw new TypeError('At least 2 arguments are required');
}
var old = arguments[0] / 1;
var current = arguments[1] / 1;
if(isNaN(old))
{
throw new TypeError('Invalid number on position 0');
}
if(isNaN(current))
{
throw new TypeError('Invalid number on position 1');
}
if(old > current)
{
return false;
}
old = current;
for (var i = 2; i < length; i++)
{
current = arguments[i] / 1;
if(isNaN(current))
{
throw new TypeError('Invalid number on position ' + i);
}
if(old > current)
{
return false;
}
old = current;
}
return true;
}
Alternatively, you can use a do{...}while loop, like this, to dry the code:
function isIncreasingSequence() {
/**Check if numbers sequence is increasing
* @param {number} numbers - a sequence of input numbers;
* @return {boolean} - true if given sequence is increasing, false othrewise
*/
var length = arguments.length;
if(length < 2)
{
throw new TypeError('At least 2 arguments are required');
}
var old = arguments[0] / 1;
if(isNaN(old))
{
throw new TypeError('Invalid number on position 0');
}
var i = 1;
var current;
do
{
current = arguments[i] / 1;
if(isNaN(current))
{
throw new TypeError('Invalid number on position ' + i);
}
if(old > current)
{
return false;
}
old = current;
}
while(++i < length);
return true;
}
Another way to improve the performance is to use old !== old
and current !== current
instead of isNaN(...)
, but the difference is neglectible.
This final version, using the do...while loop takes exactly the same time as @Kruga's ES6 answer.
numbers
to be an array, to be checked instead of thenumArr
/arguments
. Which do you want? \$\endgroup\$var num = 0
before the while. \$\endgroup\$