My answer is additive to the others; I'm not going to repeat points already made.
1. Nitpick: prefer int[] arr over int arr[]
The "arrayness" ([]) has more to do with the type than it does the variable name, so it's better to keep them together. Not to appeal to authority, but Google's style guide also suggests this.
2. Avoid "for" loops where "for-each" loops are applicable.
You never really need the indices i and j, aside from using them to get the elements. Just avoid them altogether:
for (int a : arr) {
for (int b : arr) {
if (Math.sqrt(a) == b) {
s += a + "," + b + " ";
}
}
}
Even if a for-each loop wasn't available (e.g. in C), I would still recommend extracting repeated terms (arr[i], arr[j]) to a variable.
3. Don't intermix computation of a result, and the printing of the result
I would advise against this cross-over of computing and printing that you're doing.
Perhaps you want to use this list as an input to some other process. Parsing the string back would be both inconvenient and inefficient. It's better to make systems that shuttle around machine-usable values (e.g. a list of numbers) as much as possible, and only formatting and printing them as the last possible moment.
Here's a first approximation:
import java.util.List;
import java.util.ArrayList;
class Untitled {
public static void main(String[] args) {
int arr[] = {5,25,3,25,4,2,25};
List<Integer> discoveredRoots = new ArrayList<>();
for (int a : arr) {
for (int b : arr) {
if (Math.sqrt(a) == b) {
discoveredRoots.add(b);
}
}
}
String s = "";
for (int root : discoveredRoots) {
int perfectSquare = root * root;
s += root + "," + perfectSquare + " ";
}
System.out.println(s);
}
}
4. Split off distinct chunks of code
Now we have two distinct chunks of our code within the main function. These are clearly two new functions, just waiting to get out:
class Untitled {
public static void main(String[] args) {
int[] input = {5,25,3,25,4,2,25};
List<Integer> discoveredRoots = findPerfectSquarePairs(input);
String formattedResult = formatPerfectSquarePairs(discoveredRoots);
System.out.println(formattedResult);
}
// Returns a stream of factors whose squares (which are perfect squares)
// were found in `input`.
static List<Integer> findPerfectSquarePairs(int[] input) {
List<Integer> discoveredRoots = new ArrayList<>();
for (int a : input) {
for (int b : input) {
if (Math.sqrt(a) == b) {
discoveredRoots.add(b);
}
}
}
return discoveredRoots;
}
static String formatPerfectSquarePairs(List<Integer> discoveredRoots) {
String s = "";
for (Integer root : discoveredRoots) {
int perfectSquare = root * root;
s += root + "," + perfectSquare + " ";
}
return s;
}
}
These functions have the added benefit of being pure. They don't do anything besides process over their parameters, and return a value (e.g. they don't print anything directly).
* The lack of side-effects make these methods trivial to unit unit test. Just provide some hard-coded parameters, and assert that their equality to some hard-coded results.
* Their separation make isolation trivial. You can test printing without processing, and processing without printing.
5. Optimize the internals of findPerfectSquarePairs and formatPerfectSquarePairs.
Now that we've "discovered" these two functions, we can improve each component separately. We're able to lean on unit tests to ensure we didn't break anything.
First, we can use a Set to remember which numbers we've seen, without having to do a O(input.length * 2) search over every possible pair of numbers. We don't even have to put every number in the set, only those which are perfect squares.
/*
import static java.lang.Math.pow;
import static java.lang.Math.sqrt;
import static java.lang.Math.round;
*/
static boolean isPerfectSquare(int i) {
if (i < 0) return false; // negative numbers can't be perfect squares
double root = sqrt(i);
double squared = pow(round(root), 2);
return round(squared) == i;
}
static List<Integer> findPerfectSquarePairs(int[] input) {
Set<Integer> uniqueNumbers = new HashSet<>();
for (int a: input) {
if (isPerfectSquare(a)) uniqueNumbers.add(a);
}
List<Integer> discoveredRoots = new ArrayList();
for (int a : uniqueNumbers) {
int perfectSquare = a * a;
if (uniqueNumbers.contains(perfectSquare)) {
discoveredRoots.add(a);
}
}
return new ArrayList(discoveredRoots);
}
All of these loops are just a stream screaming to get out. Let's do that:
/*
import static java.lang.Math.pow;
import static java.lang.Math.sqrt;
import static java.lang.Math.round;
import static java.util.stream.Collectors.toSet;
import static java.util.stream.Collectors.toList;
*/
static List<Integer> findPerfectSquarePairs(int[] input) {
Set<Integer> perfectSquares = Arrays.stream(input)
.filter(Untitled::isPerfectSquare)
.boxed()
.collect(toSet());
List<Integer> discoveredRoots = perfectSquares.stream()
.filter (root -> {
int perfectSquare = root * root;
return perfectSquares.contains(perfectSquare);
})
.collect(toList());
return discoveredRoots;
}
Next we can turn our attention to formatPerfectSquarePairs. It too could be simplified with a stream:
static String formatPerfectSquarePairs(List<Integer> discoveredRoots) {
return discoveredRoots.stream()
.map(root -> {
int perfectSquare = root * root;
return root + "," + perfectSquare;
})
.collect(joining(" "));
}
6. Redesign the interaction of findPerfectSquarePairs and formatPerfectSquarePairs
At this point, we can realize that both of these methods use streams internally, but don't communicate to each other using streams. findPerfectSquarePairs unnecessarily collects results into a List<Integer>, only for formatPerfectSquarePairs to stream() its elements. This is unnecessary.
Returning a Stream<Integer> would use less memory (there's no need to concurrently store all discoveredRoots in memory), and would be more flexible. If consumers want a List<Integer>, they can .collect(toList()) it themselves. But now we've given them the ability to iterate over the stream, to reduce it, map it, etc.
static Stream<Integer> findPerfectSquarePairs(int[] input) {
Set<Integer> perfectSquares = Arrays.stream(input)
.filter(Untitled::isPerfectSquare)
.boxed()
.collect(toSet());
return perfectSquares.stream()
.filter (root -> {
int perfectSquare = root * root;
return perfectSquares.contains(perfectSquare);
});
}
static String formatPerfectSquarePairs(Stream<Integer> discoveredRoots) {
return discoveredRoots
.map(root -> {
int perfectSquare = root * root;
return root + "," + perfectSquare;
})
.collect(joining(" "));
}
7. Fix the formatting of the output
...to match your desired [5,25],[5,25],[2,4],[5,25] style:
static String formatPerfectSquarePairs(Stream<Integer> discoveredRoots) {
return discoveredRoots
.map(root -> {
int perfectSquare = root * root;
return root + "," + perfectSquare;
})
.map(s -> "[" + s + "]")
.collect(joining(","));
}
Final result
import java.util.Set;
import java.util.Arrays;
import java.util.stream.Stream;
import static java.lang.Math.pow;
import static java.lang.Math.sqrt;
import static java.lang.Math.round;
import static java.util.stream.Collectors.toSet;
import static java.util.stream.Collectors.joining;
class Untitled {
public static void main(String[] args) {
int[] input = {5,25,3,25,4,2,25};
Stream<Integer> discoveredRoots = findPerfectSquarePairs(input);
String formattedResult = formatPerfectSquarePairs(discoveredRoots);
System.out.println(formattedResult);
}
static boolean isPerfectSquare(int i) {
if (i < 0) return false; // negative numbers can be perfect squares
double root = sqrt(i);
double squared = pow(round(root), 2);
return round(squared) == i;
}
static Stream<Integer> findPerfectSquarePairs(int[] input) {
Set<Integer> perfectSquares = Arrays.stream(input)
.filter(Untitled::isPerfectSquare)
.boxed()
.collect(toSet());
return perfectSquares.stream()
.filter (root -> {
int perfectSquare = root * root;
return perfectSquares.contains(perfectSquare);
});
}
static String formatPerfectSquarePairs(Stream<Integer> discoveredRoots) {
return discoveredRoots
.map(root -> {
int perfectSquare = root * root;
return root + "," + perfectSquare;
})
.map(s -> "[" + s + "]")
.collect(joining(","));
}
}
```
