public/**
static int[] findTwoSum(int[] numbers, int* target)A helper class to facilitate the use of the binary search methods of the
* {@code Arrays} class when searching for an {@code int} in an array where
* every number int[]of numbersSortedthat =array Arrays.copyOfis bound to another {@code int} (numbers,in numbers.length);this case,
* an index Arrays.sort(numbersSortedin another, unsorted array);
.
*/
List<Integer>private numberPairstatic =class newNumberWithIndex ArrayList<>();implements Comparable<NumberWithIndex> {
for/**
(int i = 0; i < numbersSorted.length && numbersSorted[i]* <=This targetfield /is 2;non-final i++)so {that a search template can be used and a
* new object intdoes jnot =have Arrays.binarySearch(
to be created for every search
*/
numbersSorted, private int number;
/**
* This field iis +ignored 1in equals, hashCode and compareTo
*/
private final int numbersSorted.length,index;
NumberWithIndex(int number, int index) {
target - numbersSorted[i]); this.number = number;
this.index = index;
}
if (j >= 0 void setNumber(int number) {
this.number = number;
numberPair.add }
int getNumber(numbersSorted[i]); {
return number;
numberPair.add }
int getIndex(numbersSorted[j]); {
return index;
break; }
@Override
} public int hashCode() {
return number;
}
/*@Override
The numbers themselves are found, now get theirpublic boolean equals(Object obj) {
indices inif the(this original== arrayobj) */{
return true;
}
if (numberPair.isEmpty!(obj instanceof NumberWithIndex)) {
return null;false;
} else { }
assertfinal numberPair.sizeNumberWithIndex other = (NumberWithIndex) obj;
return this.number == 2;other.number;
}
@Override
List<Integer> originalIndexes = new ArrayList<>public int compareTo(NumberWithIndex o); {
return this.number - o.number;
}
}
public static int[] findTwoSum(int[] numbers, int target) {
NumberWithIndex[] numbersWithOriginalIndexesSorted = new NumberWithIndex[numbers.length];
for (int i = 0; i < numbers.length && originalIndexes.size() < 2;length; i++) {
numbersWithOriginalIndexesSorted[i] = ifnew (numberPair.containsNumberWithIndex(numbers[i]), i);
{ }
Arrays.sort(numbersWithOriginalIndexesSorted);
NumberWithIndex searchTemplate = new originalIndexes.addNumberWithIndex(i0, 0);
// index of searchTemplate is irrelevant, 0 is completely arbitrary
for numberPair.remove(Integerint i = 0;
i < numbersWithOriginalIndexesSorted.valueOf(numbers[i]));length
&& numbersWithOriginalIndexesSorted[i].getNumber() <= target /* 2;
i++) {
^
searchTemplate.setNumber(target - numbersWithOriginalIndexesSorted[i].getNumber());
int j = Arrays.binarySearch(
explicit boxing necessary, otherwisenumbersWithOriginalIndexesSorted,
i + 1,
List.remove(int) will be called
numbersWithOriginalIndexesSorted.length,
*/searchTemplate);
if (j >= 0) }{
}
return new int[]{
assert originalIndexes numbersWithOriginalIndexesSorted[i].sizegetIndex(),
== 2;
assert numberPair numbersWithOriginalIndexesSorted[j].isEmptygetIndex();
return new int[]{originalIndexes.get(0), originalIndexes.get(1)};
}
}
return null;
}
This basically does what you presumably intended to do originally, but since it takes advantage of native methods like the binary search algorithms, it is probably much faster than your original code.
Update
I've refined the code that demonstrates the usage of the binary search algorithm provided by the Arrays
class. Instead of simply creating a copy of the original array, sorting that copy, finding a number pair from that sorted copy and then iterating through the original array to get the indexes, the updated code binds the original indexes to the numbers prior to sorting them. Unfortunately, this cannot be done with a Map.Entry<Integer, Integer>
, because then it would not be possible to use the binary search methods in Arrays
, since, for one thing, a Map.Entry
is not comparable, and for another, its equals(Object)
method also takes into account the entry's value, which in our case would represent the index in the original array, which is irrelevant when just searching for the numbers. It is therefore necessary to design a helper class that ignores the index the numbers are bound to in its equals
and compareTo
methods. This might seem convoluted, but from a purely programming-logical point of view, this is the most elegant solution I can think of for your algorithm (note that there might still be faster algorithms – I'm just trying to optimize the implementation of your algorithm).
public static int[] findTwoSum(int[] numbers, int target) {
int[] numbersSorted = Arrays.copyOf(numbers, numbers.length);
Arrays.sort(numbersSorted);
List<Integer> numberPair = new ArrayList<>();
for (int i = 0; i < numbersSorted.length && numbersSorted[i] <= target / 2; i++) {
int j = Arrays.binarySearch(
numbersSorted,
i + 1,
numbersSorted.length,
target - numbersSorted[i]);
if (j >= 0) {
numberPair.add(numbersSorted[i]);
numberPair.add(numbersSorted[j]);
break;
}
}
/* The numbers themselves are found, now get their
indices in the original array */
if (numberPair.isEmpty()) {
return null;
} else {
assert numberPair.size() == 2;
List<Integer> originalIndexes = new ArrayList<>();
for (int i = 0; i < numbers.length && originalIndexes.size() < 2; i++) {
if (numberPair.contains(numbers[i])) {
originalIndexes.add(i);
numberPair.remove(Integer.valueOf(numbers[i]));
/* ^
explicit boxing necessary, otherwise
List.remove(int) will be called
*/
}
}
assert originalIndexes.size() == 2;
assert numberPair.isEmpty();
return new int[]{originalIndexes.get(0), originalIndexes.get(1)};
}
}
This basically does what you presumably intended to do originally, but since it takes advantage of native methods like the binary search algorithms, it is probably much faster than your original code.
/**
* A helper class to facilitate the use of the binary search methods of the
* {@code Arrays} class when searching for an {@code int} in an array where
* every number of that array is bound to another {@code int} (in this case,
* an index in another, unsorted array).
*/
private static class NumberWithIndex implements Comparable<NumberWithIndex> {
/**
* This field is non-final so that a search template can be used and a
* new object does not have to be created for every search
*/
private int number;
/**
* This field is ignored in equals, hashCode and compareTo
*/
private final int index;
NumberWithIndex(int number, int index) {
this.number = number;
this.index = index;
}
void setNumber(int number) {
this.number = number;
}
int getNumber() {
return number;
}
int getIndex() {
return index;
}
@Override
public int hashCode() {
return number;
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (!(obj instanceof NumberWithIndex)) {
return false;
}
final NumberWithIndex other = (NumberWithIndex) obj;
return this.number == other.number;
}
@Override
public int compareTo(NumberWithIndex o) {
return this.number - o.number;
}
}
public static int[] findTwoSum(int[] numbers, int target) {
NumberWithIndex[] numbersWithOriginalIndexesSorted = new NumberWithIndex[numbers.length];
for (int i = 0; i < numbers.length; i++) {
numbersWithOriginalIndexesSorted[i] = new NumberWithIndex(numbers[i], i);
}
Arrays.sort(numbersWithOriginalIndexesSorted);
NumberWithIndex searchTemplate = new NumberWithIndex(0, 0);
// index of searchTemplate is irrelevant, 0 is completely arbitrary
for (int i = 0;
i < numbersWithOriginalIndexesSorted.length
&& numbersWithOriginalIndexesSorted[i].getNumber() <= target / 2;
i++) {
searchTemplate.setNumber(target - numbersWithOriginalIndexesSorted[i].getNumber());
int j = Arrays.binarySearch(
numbersWithOriginalIndexesSorted,
i + 1,
numbersWithOriginalIndexesSorted.length,
searchTemplate);
if (j >= 0) {
return new int[]{
numbersWithOriginalIndexesSorted[i].getIndex(),
numbersWithOriginalIndexesSorted[j].getIndex()
};
}
}
return null;
}
This basically does what you presumably intended to do originally, but since it takes advantage of native methods like the binary search algorithms, it is probably much faster than your original code.
Update
I've refined the code that demonstrates the usage of the binary search algorithm provided by the Arrays
class. Instead of simply creating a copy of the original array, sorting that copy, finding a number pair from that sorted copy and then iterating through the original array to get the indexes, the updated code binds the original indexes to the numbers prior to sorting them. Unfortunately, this cannot be done with a Map.Entry<Integer, Integer>
, because then it would not be possible to use the binary search methods in Arrays
, since, for one thing, a Map.Entry
is not comparable, and for another, its equals(Object)
method also takes into account the entry's value, which in our case would represent the index in the original array, which is irrelevant when just searching for the numbers. It is therefore necessary to design a helper class that ignores the index the numbers are bound to in its equals
and compareTo
methods. This might seem convoluted, but from a purely programming-logical point of view, this is the most elegant solution I can think of for your algorithm (note that there might still be faster algorithms – I'm just trying to optimize the implementation of your algorithm).
public static int[] findTwoSum(int[] numbers, int target) {
int[] numbersSorted = Arrays.copyOf(numbers, numbers.length);
Arrays.sort(numbersSorted);
List<Integer> numberPair = new ArrayList<>();
for (int i = 0; i < numbersSorted.length && numbersSorted[i] <= target / 2; i++) {
int j = Arrays.binarySearch(
numbersSorted,
i+1i + 1,
numbersSorted.length,
target - numbersSorted[i]);
if (j >= 0) {
if numberPair.add(jnumbersSorted[i]);
>= 0 numberPair.add(numbersSorted[j]);
{ break;
}
}
/* The numbers themselves are found, now get their
indices in the original array */
if (numberPair.isEmpty()) {
return null;
} else {
assert numberPair.size() == 2;
List<Integer> originalIndexes = new int[]ArrayList<>();
for (int i = 0; i < numbers.length && originalIndexes.size() < 2; i++) {
if (numberPair.contains(numbers[i])) {
originalIndexes.add(i,);
j} numberPair.remove(Integer.valueOf(numbers[i]));
/* ^
explicit boxing necessary, otherwise
List.remove(int) will be called
*/
}
}
assert originalIndexes.size() == 2;
assert numberPair.isEmpty();
return null;new int[]{originalIndexes.get(0), originalIndexes.get(1)};
}
}
public static int[] findTwoSum(int[] numbers, int target) {
int[] numbersSorted = Arrays.copyOf(numbers, numbers.length);
Arrays.sort(numbersSorted);
for (int i = 0; i < numbersSorted.length && numbersSorted[i] <= target / 2; i++) {
int j = Arrays.binarySearch(
numbersSorted,
i+1,
numbersSorted.length,
target - numbersSorted[i]);
if (j >= 0) {
return new int[]{i, j};
}
}
return null;
}
public static int[] findTwoSum(int[] numbers, int target) {
int[] numbersSorted = Arrays.copyOf(numbers, numbers.length);
Arrays.sort(numbersSorted);
List<Integer> numberPair = new ArrayList<>();
for (int i = 0; i < numbersSorted.length && numbersSorted[i] <= target / 2; i++) {
int j = Arrays.binarySearch(
numbersSorted,
i + 1,
numbersSorted.length,
target - numbersSorted[i]);
if (j >= 0) {
numberPair.add(numbersSorted[i]);
numberPair.add(numbersSorted[j]);
break;
}
}
/* The numbers themselves are found, now get their
indices in the original array */
if (numberPair.isEmpty()) {
return null;
} else {
assert numberPair.size() == 2;
List<Integer> originalIndexes = new ArrayList<>();
for (int i = 0; i < numbers.length && originalIndexes.size() < 2; i++) {
if (numberPair.contains(numbers[i])) {
originalIndexes.add(i);
numberPair.remove(Integer.valueOf(numbers[i]));
/* ^
explicit boxing necessary, otherwise
List.remove(int) will be called
*/
}
}
assert originalIndexes.size() == 2;
assert numberPair.isEmpty();
return new int[]{originalIndexes.get(0), originalIndexes.get(1)};
}
}
In the following, I am going to propose a few alternative, simpler notations for single statements in your code, even though some of these examples will be irrelevant after following mdfst13's suggestions because they can be replaced together with other statements by one, simpler statement.
First, let's take this line:
int stop = mod > 0 ? half+1 : half;
Since mod
can only be either 0
or 1
, the above statement can be rewritten as:
int stop = half + mod;
Also, the declarations of
mod
,half
andstop
could be inside theif(listSize >= 2)
clause, seeing as they have no relevance outside it.Your inner
for
loop can be written as:for(int otherIndex=currentIndex + 1; otherIndex<listSize; otherIndex++) { // ... }
This eliminates the need for the if
-clause inside it.
- mdfst13 has already provided a test case where your program fails, so I'll just point out the mistake in your logic for the sake of completeness. There is no reason to believe that, if a pair of numbers exists in the array that sums up to a given number, one of the two numbers must be in the first half of the array. First of all, there is no guarantee that the array is even sorted. And second, even if it were sorted, this doesn't mean that the middle element in the array (or the leftmost element of the right half, if the number of elements is odd) is equal to or greater than the half of the given number. Consider this array: [6,7,8,9,10,11], and the number 21. I think this demonstrates the flaw in your logic quite clearly. What you probably meant to do is iterate through the array until you encounter a number that is equal to or greater than the half of the sum-number. But even then, you'd have to make sure that the array is sorted first.
For implementing such an algorithm, the binarySearch
methods from the Arrays
class could prove useful:
public static int[] findTwoSum(int[] numbers, int target) {
int[] numbersSorted = Arrays.copyOf(numbers, numbers.length);
Arrays.sort(numbersSorted);
for (int i = 0; i < numbersSorted.length && numbersSorted[i] <= target / 2; i++) {
int j = Arrays.binarySearch(
numbersSorted,
i+1,
numbersSorted.length,
target - numbersSorted[i]);
if (j >= 0) {
return new int[]{i, j};
}
}
return null;
}
This basically does what you presumably intended to do originally, but since it takes advantage of native methods like the binary search algorithms, it is probably much faster than your original code.