10
\$\begingroup\$

The objective was to see what numbers can be multiplied to equal 100.

Example:

1 x 1 = 1 //no match
5 x 20 = 100 //match

The objective (as I'm learning) was only to use an array as I wanted to practice how it functioned. There are probably other classes I could have used which would have been better, for example, an ArrayList to store a match rather than an array defined of [100].

I'm pretty new to programming and would like your opinion on my following block of code.

public class ArraySearchTest
{

    public ArraySearchTest()
    {
        doTest();
    }

    public void doTest()
    {
        int [] array = new int[100];
        int [] bbb = new int[11];
        int [] ans = new int[100];

        for(int i = 0; i < array.length; i++)
        {
            for(int p = 0; p < bbb.length; p++)
            {
                if( p * i == 100)
                {
                    ans[p] = (p + i);
                    System.out.println(p + " x " + i + " Equals 100");
                }   
            }     
        }
    }
}
\$\endgroup\$
8
\$\begingroup\$

General Style:

Use functions where you can. You have embedded all the code in to one method.

For many reasons, you should create a function which takes the 100 value as a parameter. Additionally, the purpose of the code is to return pairs of numbers that have 100 as the product, so what you want is a container to store each pair.

This is a start to turning your code in to Object Oriented code.

Classes (OOP)

A container for a result, which is two values that multiply to the target, would be called something like CoFactors (Factors which together make the target). It would look something like:

public class CoFactor {
    private final int lesser, greater;

    public CoFactor(int lesser, int greater) {
        this.lesser = lesser;
        this.greater = greater;
    }

    public int getLesser() {
        return lesser;
    }

    public int getGreater() {
        return greater;
    }
}

Algorithm

Now, with that class, we can return the factors of a number with a method like:

public static final List<CoFactor> getCoFactors(int target) {
    ....
}

The above method will return a list of factors that have the target as a product.

How would that be coded? First, some math....

  • each factor of a number has a partner (perhaps, if the number is a square number, the partner is itself (3 x 3 = 9 for example) The lesser factor will only be smaller when the lesser factor is less than the square-root of the target....
  • each small factor has a partnering large factor, and there is only one partner.
  • we only need to find the small ones to also be able to find the large ones.
  • we can tell if a number is a factor if there's no remainder after a division (use the % modulo operator)

This can be computed as:

List<CoFactor> factors = new ArrayList<>();
int root = (int)Math.sqrt(target);
for (int attempt = 1; attempt <= root; i++) {
    if (target % attempt == 0) {
        factors.add(new CoFactor(attempt, target / attempt));
    }
}
return factors;
\$\endgroup\$
  • \$\begingroup\$ Thanks for the response. I have noticed you have used getters/setters in the first class and have used a second class to create an array which will then calculate the numbers and look for a remainder. If a remainder exists then it won't be added to the array. Your method seems clever (and a little complicated to me to get my head around!) but I will give it a try. Thanks for the advice. \$\endgroup\$ – bs3ac Dec 16 '14 at 0:11
7
\$\begingroup\$

PS. the objective (as I'm learning) was only to use an array as I wanted to practice how it functioned.

You lost me. You're using 3 arrays, but two are essentially unused and the third gets only written, never read. So you could drop them all.

public class ArraySearchTest
{

In Java the brace belongs on the previous line.

int [] array = new int[100];

Write int[] array as the type is int[], so it belongs together.


After dropping the arrays, not much is left.... I'd suggest to modify the task, so that an array gets really needed. The simplest possibility is probably

For a given two equally long arrays, compute how many pairs multiply to 100.

The building of the pairs is ambiguous, so these are actually 2 different tasks.

\$\endgroup\$
4
\$\begingroup\$
public class FactorFinder
{
    //research "Magic Numbers" for my reasoning on adding this.
    private final int DEFAULTNUMBER = 100

    //Pass in any integer to find it's factors (not just 100)
    public FactorFinder(int number) 
    {       
        printFactors(number); //more descriptive than doTest()
    }

    //Default constructor, use DEFAULTNUMBER to do the calculation
    public FactorFinder()
    {
        printFactors(DEFAULTNUMBER);
    }


    //changed name from doTest() to printFactors(int number)
    public static void printFactors(int number)
    {
        for(int i = 1; i <= number / 2; i++)
        {
            for(int j = 1; j <= i; j++) //changed p to j.
            {
                if( i * j == number)
                {
                    System.out.println(j + " x " + i + " Equals 100");
                    break; //This breaks out of the inner loop.
                }
                else if(i * j > number){
                    //If the product is greater than the number,
                    //there is no need to continue calculating,
                    //break out of the inner loop
                    break;
                }
            }     
        }
    }
}

Things that I did:

  • Made your code work for any positive integer you pass in, other than 100.
  • Made 100 the default number in-case you did not specify the number.
  • Improved the efficiency of the loop. If you do the math, everything should check out:

    • started index at 1 because 0 x number = 0 : wasted loop time
    • Outer loop: (i < number / 2) : do the math and think about why I did this.
    • loop: (j <= i) : This will eliminate double calculations
    • 20 X 5 = 100 and 5 X 20 = 100 (This depends on the problem though)
    • added continue break statements that leave the inner loop if the product is greater.
\$\endgroup\$
  • 2
    \$\begingroup\$ There's no point in constructing an object. Just make printFactors(number) a static function. \$\endgroup\$ – 200_success Dec 16 '14 at 3:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.