Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I had faced a interview question to find the gdc(greatest common divisor) for an integer array of element in optimised way :

Sample case :
   a[] = { 10,20,19,15,13}

Result = 1

sample case : 
  a[]={9,21,12,15,27}
Result : 3

I have submitted following result during the interview. But he asked to optimise the same. Solution which I proposesd:

package threeDpalm;

import java.util.Arrays;

public class GCF {
public static void main(String ag[]){
    GCF gcf = new GCF();
    gcf.check();
}
void check(){
    int[] input = {40,52};
    int [] original =input.clone();
    boolean flag = true; 
    int min_araay = Integer.MAX_VALUE;
    int countResult=0;
    int count=0;
    int k1=0;

    while (flag){

        countResult=0;
        count=0;
        //finding the minimum no which is greater than zero
        for (int j : input){
            if(j<min_araay && j!=0){
                min_araay = j;
                    }
                }
        // Applying Elucid algo...diving the lowesst elemt with other to remove the multiple of lowest
        for (int k=0;k<input.length; k++){
                input[k]= input [k]%min_araay;
                // if all the element are Zero, then it is the GCD..counting the Zero... 
                if(input[k]==0){
                count++;
                //if count = length ..breaking the loop
                if(count == input.length){
                    flag =false;
                            }
                        }
                }


        }
    //checking the result by diving each elemt of the array. cheking for all     the no less than that 
    for ( k1= min_araay;k1>0;k1--){
        countResult=0;
        for (int j : original){
            if (j%k1==0){
            countResult++;}}
            if (countResult==original.length){
                System.out.println(k1);
                return;
            }


        }

      }
   }

Can some one please help me with better solution

share

locked by Jamal Jun 20 at 18:32

This post has been locked while disputes about its content are being resolved. For more info visit meta.

closed as off-topic by nhgrif, Martin R, RubberDuck, Mast, Simon Forsberg Jun 20 at 13:32

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – nhgrif, Martin R, RubberDuck, Mast, Simon Forsberg
If this question can be reworded to fit the rules in the help center, please edit the question.

    
Unfortunately, your algorithm is wrong. Examples: gcd(3, 5) = 1, but your program prints 2. gcd(12, 20, 30) = 2, but your program prints 6. –  Martin R Jun 20 at 11:06
1  
Given that you have a hard-coded input value and your code doesn't work in at least some special cases, I'm going to assume you did not test this for any scenarios besides your single hard-coded case. As such, I'm voting to close this question as containing broken code. –  nhgrif Jun 20 at 11:21
    
@MartinR : HI guys thanx for the bug. Can you please check it again... and help me out to write better solution –  vinod bazari Jun 20 at 12:32
1  
Sorry, but still not working: gcd(6, 10) = 2, but your program prints 1. – Perhaps some comments in your code how the algorithm is supposed to work would be helpful. –  Martin R Jun 20 at 13:22
    
A better approach was suggested to you as an answer to your SO question stackoverflow.com/questions/28391752/…. –  Martin R Jun 20 at 13:40

1 Answer 1

I guess your code is based on Euclid's algorithm? For completeness, the idea is that if a = q * b + r then gcd(a,b) = gcd(b, r). If r==0 then you are done (output b), otherwise go on. So a recursive algorithm would be:

int gcd(int a, int b) { // check a >= b
        if (b == 0){
            return a;
        } 
        return gcd(b, a % b); 
    }

You can sort the input array and try all gcd(x1,x2) sequentially (maybe show off your knowledge of Java 8 using streams) until you check all of them or you get gcd = 1 which means no common factor exists, i.e. the idea is if you have the non increasing sequence {a1, a2, a3, ..., an} to compute gcd(...gcd(gcd(a1, a2), a3), ... , an)

share

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