Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Considering that indexOf is \$O(n)\$ indexOf is \$O(n)\$, you might want to avoid doing it twice.

Considering that indexOf is \$O(n)\$, you might want to avoid doing it twice.

Considering that indexOf is \$O(n)\$, you might want to avoid doing it twice.

Source Link
Brythan
  • 7k
  • 3
  • 21
  • 37

##Nitpicks##

function process(data){

On an interview question, I'd prefer to see an answer like

function find_pairs_that_sum_to(numbers, sum) {

That way I can see that you write readable, reusable code.

var result = []
var a;

I find code like this confusing. Why no ; on the first line? Why on the second line? I'd prefer to see consistency in the formatting. Also, I'd name result pairs.

 if ( (parseInt(a) + parseInt(b)) === 100 && result.indexOf(a+","+b) == -1 && result.indexOf(b+","+a ) == -1 ) {
 result.push( a+","+b )
 }

Considering that indexOf is \$O(n)\$, you might want to avoid doing it twice.

 b = parseInt(data[j]);
 if ( (a + b) === sum ) {
 if ( a <= b ) {
 if ( -1 == pairs.indexOf(a+","+b) ) {
 pairs.push( a+","+b );
 }
 } else {
 if ( -1 == pairs.indexOf(b+","+a) ) {
 pairs.push( b+","+a );
 }
 }
 // we don't need to try any more once we have a match
 // move to the next outer loop value
 break;
 }

Note that your algorithm is \$O(n^3)\$. One \$n\$ for each loop and a third for the indexOf. You can argue \$O(n^2m)\$ where \$m\$ is the size of the solution, but the bad case is where the solution is half as large as the input.

##Alternative##

One of the other answers mentioned using a map, and that will work. However, for working with numbers, an array may do fine. Of course, this stops working if the problem space becomes big. It works fine for a hundred values though.

function find_pairs_that_sum_to(numbers, sum) {
 var is_in_numbers = [];
 for ( var i = 0; i < sum; ++i ) {
 is_in_numbers.push(false);
 }
 for ( var i = 0; i < numbers.length; ++i ) {
 // assumes that all values in numbers are in [0, sum]
 // if that can be untrue, bad values should be caught here
 is_in_numbers[parseInt(numbers[i])] = true;
 }
 var pairs = [];
 var n = sum / 2;
 for ( var i = 0; i <= n; ++i ) {
 if ( is_in_numbers[i] && is_in_numbers[sum - i] ) {
 pairs.push([i, sum - i]);
 }
 }
}

Testing on your sample data produces almost the same result, only the order differs. This function is linear against the solution space (i.e. 101 in this case) or the number of inputs, whichever is larger. You could write that as \$O(m + n)\$

If the solution space is much larger than the number of inputs, then you can change is_in_numbers from an array to a map. That would give you a straight \$O(n)\$ solution.

default

AltStyle によって変換されたページ (->オリジナル) /