HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Generating Pythagorean triples below an upper bound

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
boundtriplespythagoreangeneratingbelowupper

Problem

I've got an assigment where I have to print all the Pythagorean triples smaller than a given n. Even though I could print them directly in pythagoreanTriple function, I decided to structure everything, as I want to go deply into programming structure.(my teacher does not teach us a lot, so I'll have to study on my own).

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

public class Main {

    public static void main(String[] args) {
        System.out.println("Starting program...");
        int n = readFromKeyboard();
        printResult(pythagoreanTriple(n));
    }

    private static int  readFromKeyboard(){
        Scanner keybInput = new Scanner(System.in);
        return keybInput.nextInt();
    }

    private static List> pythagoreanTriple(int n){
        List> pytTri = new ArrayList>();
        int i,j,k;
        for (i = 3; i  i; j--)
                for (k = n; k > j; k--)
                    if (i * i + j * j == k * k){
                        ArrayList tempArray = new ArrayList(); //we create a list of integers
                        tempArray.add(i);
                        tempArray.add(j);
                        tempArray.add(k);
                        pytTri.add(tempArray);
                    }
        return pytTri;
    }

    private static void printResult(List> resultList){
        for (List ls: resultList){
            for (Integer i: ls){
                System.out.print(i);
                System.out.print(" ");
            }
            System.out.println(";");
        }
    }

}


Knowing these:

I am curious about the following: How are the variable/function's name, should I use underscore _ for private variables etc...
I am not new to programming

(I know both C++ and Python so far), but I'm more interested in learning Java next.)

Solution

It's a good thing that there is a method to calculate the pythagorean triple up to a number, and then a second method to print the results.

About pythagoreanTriple

You can simply the pythagoreanTriple method in several aspects. First of all is performance. The current code does:

for (i = 3; i  i; j--)
        for (k = n; k > j; k--)
            if (i * i + j * j == k * k) {


to check for triplets. This tries every possible combinations of i, j and k and checks if they form a triplet. However, you don't need to loop for every possible k: once you know i and j, you can deduce the only possible k for which (i,j,k) forms a triplet; it is exactly \$k = \sqrt{i^2+j^2}\$, when this result in an integer. Take care because such a calculation can lead to rounding issues, and there are good way to check if a number square root is an integer. I won't comment more on the performance, because if this is really a concern, there are other faster approaches than trying every possible combinations (see for example the Tree of primitive Pythagorean triples).

Second aspect, is the usage of built-in methods.

ArrayList tempArray = new ArrayList<>();
tempArray.add(i);
tempArray.add(j);
tempArray.add(k);
pytTri.add(tempArray);


can be written more simply using the built-in Arrays.asList. You can directly have:

pytTri.add(Arrays.asList(i, j, k));


without creating any new ArrayList yourself.

Finally, even if it isn't strictly necessary, consider adding curly braces everywhere.

for (i = 3; i  i; j--)


can quickly lead to problems if you want to add another statement in there, like storing the result of i * i as recommented by coderodde in their answer.

About readFromKeyboard

Currently, the code to retrieve the user input does not validate that only integers can be input. If you try to input a String like "a", an exception will be thrown; consider re-prompting for an input if that happens.

Also, this method creates a new Scanner. In the current code, it is called only once, but if later you want to read a second input from the user, it would be better to create it only once and reuse it. As such, consider creating the Scanner and passing it as parameter of readFromKeyboard.

This also has a direct impact: readFromKeyboard would not read from the keyboard anymore, it would read from a given Scanner, which makes it more reusable, since you could pass any instance of Scanner you want (for example, one reading from a file). And the method should renamed to something like readUpperBound, which makes it clear that its concern is not to read from the keyboard (which is the current technical implementation), but to fetch the upper bound to use in order to create the triplets (which is what it's really supposed to do).

Code Snippets

for (i = 3; i < n; i++)
    for (j = n; j > i; j--)
        for (k = n; k > j; k--)
            if (i * i + j * j == k * k) {
ArrayList<Integer> tempArray = new ArrayList<>();
tempArray.add(i);
tempArray.add(j);
tempArray.add(k);
pytTri.add(tempArray);
pytTri.add(Arrays.asList(i, j, k));
for (i = 3; i < n; i++)
    for (j = n; j > i; j--)

Context

StackExchange Code Review Q#145001, answer score: 5

Revisions (0)

No revisions yet.