patternjavaModerate
Array whose values are the product of every other integer
Viewed 0 times
thewhosearrayareeveryproductvaluesotherinteger
Problem
I was asked the following interview question over the phone:
Given an array of integers, produce an array whose values are the
product of every other integer excluding the current index.
Example:
and I gave the following answer
```
import java.math.BigInteger;
import java.util.Arrays;
public class ProductOfAnArray {
public static void main(String[] args) {
try {
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 3, 2, 8 })));
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 0, 2, 8 })));
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 0, 2, 0 })));
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] {})));
System.out
.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 3, 2, 8, 3, 2, 4, 6, 7,
3, 2, 4 })));
System.out
.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 3, 2, 8, 3, 2, 4, 6, 7,
3, 2, 4 })));
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4432432, 23423423, 34234, 23423428,
4324243, 24232, 2342344, 64234234, 4324247,
4234233, 234422, 234244 })));
} catch (Exception e) {
// debug exception here and log.
}
}
/*
* Problem: Given an array of integers, produce an array whose values are
* the product of every other integer excluding the current index.
*
* Assumptions. Input array cannot be modified. input is an integer array
* "prod
Given an array of integers, produce an array whose values are the
product of every other integer excluding the current index.
Example:
[4, 3, 2, 8] -> [3*2*8, 4*2*8, 4*3*8, 4*3*2] -> [48, 64, 96, 24]and I gave the following answer
```
import java.math.BigInteger;
import java.util.Arrays;
public class ProductOfAnArray {
public static void main(String[] args) {
try {
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 3, 2, 8 })));
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 0, 2, 8 })));
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 0, 2, 0 })));
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] {})));
System.out
.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 3, 2, 8, 3, 2, 4, 6, 7,
3, 2, 4 })));
System.out
.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 3, 2, 8, 3, 2, 4, 6, 7,
3, 2, 4 })));
System.out.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4432432, 23423423, 34234, 23423428,
4324243, 24232, 2342344, 64234234, 4324247,
4234233, 234422, 234244 })));
} catch (Exception e) {
// debug exception here and log.
}
}
/*
* Problem: Given an array of integers, produce an array whose values are
* the product of every other integer excluding the current index.
*
* Assumptions. Input array cannot be modified. input is an integer array
* "prod
Solution
For starters, you have very weird line breaks:
That's not layout, that's obfuscation! When dealing with long method chains, it may be preferable to break before each method invocation:
but that's not the case here, and this layout does not increase readability. Instead, break after the opening paren of the argument list:
or something like that. Note that you could have written this as
(i.e. without the explicit array) when declaring your function with varargs:
The next question is why you are returning an array – a
But why
Your code inside
Notice that
This conditional in the next loop is also too complex, and could be simplified to
When applying these criticisms, the Simplest Thing That Could Work might be this:
Oh, and naming! Have vocals become so precious that we must write
Your code is not bad, but it isn't good, and could certainly have been written better. The next time, maybe :)
System.out
.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 3, 2, 8, 3, 2, 4, 6, 7,
3, 2, 4 })));That's not layout, that's obfuscation! When dealing with long method chains, it may be preferable to break before each method invocation:
someObject
.thisIs()
.aFluid()
.interface();but that's not the case here, and this layout does not increase readability. Instead, break after the opening paren of the argument list:
System.out.println(
Arrays.toString(ProductOfAnArray.calcArray(new int[] {
4, 3, 2, 8, 3, 2, 4, 6, 7, 3, 2, 4
}))
);or something like that. Note that you could have written this as
System.out.println(
Arrays.toString(ProductOfAnArray.calcArray(
4, 3, 2, 8, 3, 2, 4, 6, 7, 3, 2, 4
))
);(i.e. without the explicit array) when declaring your function with varargs:
calcArray(int... inp).The next question is why you are returning an array – a
List would be more flexible – arrays have little use in modern Java (the usage of varargs implies arrays, and high-performance code may need them. But in all other cases, they are too inflexible – e.g. arrays and generics don't work together).But why
BigInteger? Yes, you don't want to overflow, but we can assume that your interviewers would be content with an int. If you want to allow larger numbers, the first step would be long. Big Integers are truly awkward to use and necessarily less performant that native types, so I would avoid them wherever possible.Your code inside
calcArray is unnecessarily clever. Let's look at this loop:for (int i : inp) {
if (i == 0) {
cnt++;
foundZero = true;
if (cnt < 2)
continue;
else
break;
}
multiple = multiple.multiply(BigInteger.valueOf(i));
}Notice that
foundZero is equivalent to cnt > 0, and that you aren't using braces around the bodies of the inner if/else. Your continue is rather confusing, and the following expresses the control flow much better:for (int i : inp) {
if (i != 0) {
multiple = multiple.multiply(BigInteger.valueOf(i));
}
else {
cnt++;
if (cnt >= 2) {
break;
}
}
}This conditional in the next loop is also too complex, and could be simplified to
if (cnt < 2 && inp[i] == 0) {
ans[i] = multiple;
} else {
ans[i] = new BigInteger("0");
}When applying these criticisms, the Simplest Thing That Could Work might be this:
public static List specialProduct(int... numbers) {
long product = 1;
int zeroesCount = 0;
for (int n : numbers) {
if (n != 0) {
product *= n;
}
else {
zeroesCount++;
}
}
List output = new ArrayList<>(numbers.length);
switch (zeroesCount) {
case 0:
// with no zeroes, we don't have to perform any checks
for (int n : numbers) {
output.add(product / n);
}
break;
case 1:
// with one zero, the all others will become zero too
for (int n : numbers) {
if (n == 0) {
output.add(product);
}
else {
output.add(0L);
}
}
break;
default:
// two or more zeroes make everything zero
for (int i = 0; i < numbers.length; i++) {
output.add(0L);
}
break;
}
return output;
}Oh, and naming! Have vocals become so precious that we must write
cnt instead of count? Must we write ans instead of answer if you can actually write a long word like multiple? And what the heck is inp?Your code is not bad, but it isn't good, and could certainly have been written better. The next time, maybe :)
Code Snippets
System.out
.println(Arrays.toString(ProductOfAnArray
.calcArray(new int[] { 4, 3, 2, 8, 3, 2, 4, 6, 7,
3, 2, 4 })));someObject
.thisIs()
.aFluid()
.interface();System.out.println(
Arrays.toString(ProductOfAnArray.calcArray(new int[] {
4, 3, 2, 8, 3, 2, 4, 6, 7, 3, 2, 4
}))
);System.out.println(
Arrays.toString(ProductOfAnArray.calcArray(
4, 3, 2, 8, 3, 2, 4, 6, 7, 3, 2, 4
))
);for (int i : inp) {
if (i == 0) {
cnt++;
foundZero = true;
if (cnt < 2)
continue;
else
break;
}
multiple = multiple.multiply(BigInteger.valueOf(i));
}Context
StackExchange Code Review Q#45498, answer score: 17
Revisions (0)
No revisions yet.