Problem 9 (Project Euler) with Java - is it perfect program?

Answer 1

Here is how I would re-work your program:

public class m  {
    public static void main (String[] args) {
        int nes_sum = 1000; //necessary sum
        int last_number = 998; //let's count up to 998

        //let's do bruteforce
        for(int i = 1; i < last_number; i++) {
            for(int o = i+1; o < last_number; o++) {
                for(int p = o+1; p < last_number; p++) {
                    check_everything n = new check_everything(i, o, p); //creating class

                    if(n.is_sum() && n.is_pythagor()) {
                        n.post_numbers();
                        return;
                    }
                }                
            }         
        }
    }
}

class check_everything //class for checking if it is Pythagoras and sum for 3 numbers {
    int need_sum = 1000;
    int first_number;
    int second_number;
    int third_number;

    public check_everything(int i, int j, int k) {
        setNumbers(i, j, k);
    }

    private void setNumbers(int a, int b, int c) {
        first_number = a;
        second_number = b;
        third_number = c;
    }

    public boolean is_pythagor() {
        return (first_number * first_number + second_number * second_number == third_number * third_number);  
    }

    public boolean is_sum() {
        return (first_number + second_number + third_number == need_sum);
    }

    public void post_numbers() {
        int product = first_number * second_number * third_number;
        System.out.println("We have numbers! Product is: "+ first_number +" * "+ second_number +" * "+ third_number +" = "+ product);
    }
}

The biggest change is the method's return types. Instead of setting internal variables, it is way better to return a value.

Also, to 'break' all of the way out of the program, just use return.

Answer 2

Your check_everything class is more like a little side-effect container. Me? I'd personally change it to be something along the lines of

public class check_satisfactory{
    private boolean isPythagor, isSum;

    public check_satisfactory(int a, int b, int c, int needed){
        isPythagor = pythagor_check( a, b, c );
        isSum = sum_check( a, b, c, needed );
    }

   boolean getConditional(){ return isPythagor && isSum; }

   //with functions issuing return values
}

In short, the critique I have of your check_everything class is that I don't know if the functions have been called before or after I'm issued it. If you were to hand me one, I'd need to call all the functions in the order they have to be called to arrive at the solution I wanted. This would entail me knowing something about the inner workings of your class, rather than your class providing me functionality that was at my finger tips.

OOP involves encapsulation of ideas, data hiding, and should strive to be clear of purpose. I hope that didn't come off as too harsh. You're off to a good start.

Answer 3

I see three kinds of problems in your code:

1. Computational complexity too big (language-agnostic remark)
2. Low "object-orientedness"
3. Java code conventions broken

Computational complexity

Imagine that the constant 1000 in the task is changed to 100000. Your approach will no longer be feasible, since you're making roughly N³/6 checks (N is the constant). The most obvious optimization is based on the observation that there are two degrees of freedom here: once you have the first two numbers (a and b) you don't have to loop over the entire range to find c, since it's simply 1000 - a - b. The following loops will suffice (with roughly N²/6 checks):

for (int i = 1; i <= lastNumber; i++) {
    for (int o = i + 1; o <= (necessarySum - i) / 2; o++) {
        p = necessarySum - i - o;
        // Proceed with checks, knowing that i < o <= p and i + o + p == necessarySum
    }
}

Note that this is still a brute-force approach.

Object-orientedness

Class instances represent "objects" (a triple) rather than "actions" (checking a triple), so since you're dealing with triples, it would be reasonable to have a Triple class. Let's note that there's no need to modify the instances once they're created (i.e., the instances may be immutable). For a given instance, you would like to 1) know whether it's Pythagorean 2) be able to get a String representation. Hence two instance methods: isPythagorean and toString (the latter overrides the toString method of Object).

public class Triple {
    private final int a, b, c;

    public Triple(int a, int b, int c) {
        if ((a >= c) || (b >= c)) {
            throw new IllegalArgumentException("c is not the largest of the three");
        }
        this.a = a;
        this.b = b;
        this.c = c;
    }

    public boolean isPythagorean() {
        return a*a + b*b == c*c;
    }

    @Override
    public String toString() {
        return "(" + a + ", " + b + ", " + c + ")";
    }
}

Java code conventions

In Java code conventions, names of Java classes start with capital letter, both class names and variable names are written using camel case, and open braces appear at the end of the same line as the declaration statement or beginning of the compound statement.

Answer 4

I'd suggest to do the math before you worry about your program. You get all triples a²+b²=c² by this formula:

a = k(m²-n²), b = k(2mn), c = k(m² + n²)

(see http://en.wikipedia.org/wiki/Pythagorean_triple)

So if you find a triple which sum divides 1000, you just multiply it by the factor k to get the real solution. Hence you need to scan only for values of m and n using

a = m²-n², b = 2mn, c = m² + n²

until you find a triple with 1000 % (a+b+c) == 0. Obviousely m must be bigger than n. So all you need is:

public class Euler009 {

    public static int solve(int s) {
       for(int m = 2; m < Integer.MAX_VALUE; m++) {
           for(int n = 1; n < m; n ++) {
               int a = m*m - n*n;
               int b = 2*m*n;        
               int c = m*m + n*n;
               if(s % (a+b+c) == 0) {
                   int k = s / (a+b+c);
                   return k*a * k*b * k*c;
               }
           }
       }        
       return -1;
    }

    public static void main(String[] args) {
        System.out.println(solve(1000));
    }
}

As you can see, there is little need for OO here. And if OO doesn't help you to get the job done, don't use it. For most "practical" problems (and some of the more advanced Euler problems) OO can help you a lot, but here you have just two simple loops, not enough to put OO to good use...