Output breaks as input parameter increase in size

Question

I am writing a basic program that takes a parameter and then calculates the product of the odd numbers based on that parameter.

It works if I pass anything from 1 up to 19 but at 20 my output is a negative number and at 35 it is 0??

I'm sure there is something wrong in the algorithm?

Suggestions:

import java.util.ArrayList;
import java.util.List;


public class ProductOfIntegers {

public static void productOfOddIntegers(int i){
    int[] numbers = new int[i];
    for(int j = 0; j < numbers.length; j++){
        numbers[j] = j + 1;
    }
    List<Integer> oddNumbers = new ArrayList<Integer>();
    for(int j = 0; j < numbers.length; j++){

        if(numbers[j] % 2 != 0){
            oddNumbers.add(numbers[j]);
        }
    }       
    int product = 1;
    for(int n: oddNumbers)
        product*=n;
    System.out.println(oddNumbers); 
    System.out.println(product);

}

public static void main(String [] args){
    productOfOddIntegers(15);
}

}

Not an answer to your problem, but `if(numbers[j] % 2 == 0)` gets even numbers. — Evan Frisch, Jun 03 '15 at 21:56
What's the maximum value of an int? What is the value of 1 * 3 * ... * 21? — JB Nizet, Jun 03 '15 at 22:00
@Evan Frisch - Thanks for pointing that out. I always get those mixed up! Fixed it in the edit. — mrmacaroo, Jun 03 '15 at 22:01
Use a long or a BigInteger. Btw, there's no need to store all numbers, just iterate through the odd numbers and multiply as you need — Iamsomeone, Jun 03 '15 at 22:05
@JB Nizet If I'm exceeding the max value why would it go to a negative number in output? — mrmacaroo, Jun 03 '15 at 22:05
Because the left-most bit, that is set to 1 when overflowing the integer, is the sign bit, making the number negative. Read http://stackoverflow.com/questions/13422259/how-are-integers-internally-represented-at-a-bit-level-in-java — JB Nizet, Jun 03 '15 at 22:08

Willem Van Onsem · Accepted Answer · 2015-06-03T22:18:50.130

calculates the product of the odd numbers based on that parameter.

No your program calculates the product of the even numbers.

This being said, your algorithm is rather inefficient: instead of first generating a an array of integers, then filtering and finally calculating the product, you can simply use a single for loop:

public static long productOfOddIntegers(int n){
    long prod = 1;
    for(int i = 3; i <= n; i += 2) {
        prod *= i;
    }
    return prod;
}

You also better use a long since such products can be rather huge. This scales with the root of a factorial after all.

You can also use a BigInteger approach for which you can use an arbitrary n:

public static BigInteger productOfOddIntegers(int n){
    BigInteger prod = BigInteger.ONE;
    BigInteger bin = new BigInteger(""+n);
    BigInteger two = BigInteger.ONE.add(BigInteger.ONE);
    for(BigInteger i = two.add(BigInteger.ONE); i.compareTo(bin) <= 0; i = i.add(two)) {
        prod = prod.multiply(i);
    }
    return prod;
}

jdoodle demo

It is furthermore bad design to do println commands in methods. You should make a separation between calculating and printing.

Output breaks as input parameter increase in size

1 Answers1