CODEWITHSUNDEEP
javalist
Kenneth Yong
Kenneth Yong

Reputation: 385

Printing and returning the correct list

I'm currently working on the below question

// It should return the greatest common factor
    // between two numbers.  
    //
    // Examples of greatestCommonFactor:
    //   greatestCommonFactor(6, 4)   // returns 2
    //   greatestCommonFactor(7, 9)   // returns 1
    //   greatestCommonFactor(20, 30) // returns 10
    //
    // Hint: start a counter from 1 and try to divide both
    // numbers by the counter. If the remainder of both divisions
    // is 0, then the counter is a common factor. Continue incrementing
    // the counter to find the greatest common factor. Use a while loop
    // to increment the counter.

My code is shown below

public static List greatestCommonFactor(int a, int b){
        int i = 1 ;
        List topnum = new ArrayList();
        ArrayList <Integer> factor = new ArrayList<Integer>();
        while (i <= a || i <= b ){
            i++;
        }
        if (a%i == 0 && b%i == 0){
            factor.add(i);  
        }
        else if (a%i <= 1 || b%i <= 1){
            Collections.sort(factor);
            List<Integer> topnum1 = factor.subList(factor.size() - 1, factor.size());

        }
        return topnum;
    }

I'm having an issue getting the right output. Currently I'm getting [] as output, which is strange. I can't seem to get topnum into the List<Integer> line without getting an error either, so this is about what I can manage.

Anyone has any tips to get me to print the elements from topnum1 to solve this tutorial?

Upvotes: 0

Views: 50

Answers (2)

Eran
Eran

Reputation: 393856

Your while loop does nothing. You should probably put your condition inside it, or it would only be tested for the last value of i.

while (i <= a || i <= b ){
    if (a%i == 0 && b%i == 0){
        factor.add(i);  
    }
    i++;
}

Beside that, you add elements to factor list (at least you would after the fix), and put the last element of that list in topnum1, but your method returns topnum which remains empty.

Finally, your else if (a%i <= 1 || b%i <= 1) is unclear to me. And you don't need to sort the factor list. It will already be sorted. Actually, you don't need that list at all, just keep the largest i that is a common factor and return it.

This will make the code much more simple :

public static int greatestCommonFactor(int a, int b)
{
    int result = 1;
    int i = 1 ;
    while (i <= a && i <= b){
        if (a%i == 0 && b%i == 0){
            result = i;
        }
        i++;
    }

    return result;
}

Upvotes: 3

Alex K
Alex K

Reputation: 8338

You don't actually change the value of topnum anywhere...

All you did is create it at the top with List topnum = new ArrayList();, then returned it at the bottom with return topnum;. You didn't add anything to it.

If you don't know how to add something to a list, you use .add():

 topnum.add(7);

That will at 7 to topnum.

Upvotes: 0

Related Questions