patternjavascriptMinor
Find if an array is arithmetic or geometric
Viewed 0 times
arrayarithmeticgeometricfind
Problem
This seems to be working fine. I am pretty sure there is a better way to do it. How can my code be improved performance-wise? The main thing bothering me is the nested
for-loop.function arithGeo (arr) {
var isGeometric = false;
var isArithmetic = false;
var diff = arr[2] - arr[1];
var ratio = arr[2] / arr[1];
for (var i = 0; i < arr.length - 1; i++) {
var j = i + 1;
if (diff !== arr[j] - arr[i]) {
for (var k = 0; k < arr.length - 1; k++) {
var l = k + 1;
if (ratio !== arr[l] / arr[k]) {
break;
} else if (l === arr.length-1) {
isGeometric = true;
break;
}
}
} else if (j === arr.length - 1) {
isArithmetic = true;
}
}
if (isGeometric === true) {
return "geometric";
} else if (isArithmetic === true) {
return "arithmetic";
}
else {
return -1;
}
}
console.log(arithGeo([2,4,6,8]));
console.log(arithGeo([2,6,18,54]));
console.log(arithGeo([2,6,1,54]));Solution
I agree with @janos that sometimes returning a string and sometimes returning
I find it odd that you base the difference and ratio on elements at indexes 1 and 2 rather than 0 and 1. Such an arbitrary choice seems like an unnecessary failure mode for short input arrays.
Note that if the ratio is 0 or infinite, you can immediately declare the sequence to be not geometric.
A better approach is to assume that a sequence is arithmetic or geometric until proven otherwise.
For clarity, you might as well write it as two independent loops, because the classification problems are independent of each other. There would be more lines of code, but the complexity of the algorithm remains the same. Except in degenerate cases, only one of the loops is going to see much action anyway.
-1 makes this function hard to use. In addition, it's unclear what your function is supposed to return for input that is both an arithmetic and a geometric sequence (every element is the same).I find it odd that you base the difference and ratio on elements at indexes 1 and 2 rather than 0 and 1. Such an arbitrary choice seems like an unnecessary failure mode for short input arrays.
Note that if the ratio is 0 or infinite, you can immediately declare the sequence to be not geometric.
A better approach is to assume that a sequence is arithmetic or geometric until proven otherwise.
function arithGeo(arr) {
var diff = arr[1] - arr[0];
var ratio = arr[1] / arr[0];
var isArithmetic = true, isGeometric = isFinite(ratio) && ratio != 0;
for (var i = 1, j = 2; (isArithmetic || isGeometric) && j < arr.length; i++, j++) {
if (isArithmetic && arr[j] - arr[i] != diff) {
isArithmetic = false;
}
if (isGeometric && arr[j] / arr[i] != ratio) {
isGeometric = false;
}
}
return isArithmetic && isGeometric ? 'both' :
isArithmetic ? 'arithmetic' :
isGeometric ? 'geometric' : '';
}For clarity, you might as well write it as two independent loops, because the classification problems are independent of each other. There would be more lines of code, but the complexity of the algorithm remains the same. Except in degenerate cases, only one of the loops is going to see much action anyway.
function arithGeo(arr) {
var i, j;
var diff = arr[1] - arr[0];
var isArithmetic = true;
for (i = 1, j = 2; isArithmetic && j < arr.length; i++, j++) {
if (isArithmetic && arr[j] - arr[i] != diff) {
isArithmetic = false;
}
}
var ratio = arr[1] / arr[0];
var isGeometric = isFinite(ratio) && ratio != 0;
for (i = 1, j = 2; isGeometric && j < arr.length; i++, j++) {
if (isGeometric && arr[j] / arr[i] != ratio) {
isGeometric = false;
}
}
return isArithmetic && isGeometric ? 'both' :
isArithmetic ? 'arithmetic' :
isGeometric ? 'geometric' : '';
}Code Snippets
function arithGeo(arr) {
var diff = arr[1] - arr[0];
var ratio = arr[1] / arr[0];
var isArithmetic = true, isGeometric = isFinite(ratio) && ratio != 0;
for (var i = 1, j = 2; (isArithmetic || isGeometric) && j < arr.length; i++, j++) {
if (isArithmetic && arr[j] - arr[i] != diff) {
isArithmetic = false;
}
if (isGeometric && arr[j] / arr[i] != ratio) {
isGeometric = false;
}
}
return isArithmetic && isGeometric ? 'both' :
isArithmetic ? 'arithmetic' :
isGeometric ? 'geometric' : '';
}function arithGeo(arr) {
var i, j;
var diff = arr[1] - arr[0];
var isArithmetic = true;
for (i = 1, j = 2; isArithmetic && j < arr.length; i++, j++) {
if (isArithmetic && arr[j] - arr[i] != diff) {
isArithmetic = false;
}
}
var ratio = arr[1] / arr[0];
var isGeometric = isFinite(ratio) && ratio != 0;
for (i = 1, j = 2; isGeometric && j < arr.length; i++, j++) {
if (isGeometric && arr[j] / arr[i] != ratio) {
isGeometric = false;
}
}
return isArithmetic && isGeometric ? 'both' :
isArithmetic ? 'arithmetic' :
isGeometric ? 'geometric' : '';
}Context
StackExchange Code Review Q#77356, answer score: 2
Revisions (0)
No revisions yet.