patternjavaMinor
Outputs all substrings that are palindromes in a given string, in alphabetical order without repitition
Viewed 0 times
substringsorderwithoutallarepalindromesrepititionthatoutputsalphabetical
Problem
I'm currently trying to find a way to get my program to run more efficiently. If anyone has any ideas on how to optimize my program, i would appreciate it a lot. Also, the output has to display in alphabetic order without any duplicates.
package Palindrome;
import java.io.*;
import java.util.*;
public class Palindrome {
public static void main(String[] args)throws IOException{
Scanner scan;
scan = new Scanner(System.in);
while (scan.hasNext()) {
subPal(scan.next());
}
if (scan != null) {
scan.close();
}}
private static void subPal(String str) {
String s1 = "";
int N = str.length();
ArrayList palindromeArray = new ArrayList();
for (int i = 2; i = N)
continue;
s1 = str.substring(j, i + j);
if (s1.equals(new StringBuilder(s1).reverse().toString())&& !palindromeArray.contains(s1)) {
palindromeArray.add(s1);
}
}
}
Collections.sort(palindromeArray);
for (String s : palindromeArray)
System.out.println(s );
}
}Solution
Performance
These are slow operations that can be replaced with faster ones:
-
-
-
-
The slowest part of all is the main algorithm of enumerating possible palindromes. The nested loop basically checks all possible substring. That's not necessary. A more clever approach would be, for each position, center on the position, and keep going outward as long as the start and end letters still form a palindrome. This way you can drastically reduce the number of comparisons made in the process of checking for palindromes.
Coding style
The coding style of your post is extremely poor.
Other issues
The
The initializer in
These are slow operations that can be replaced with faster ones:
-
s1 = str.substring(j, i + j); creates new strings. A small improvement would be to use .charAt calls to access elements of str without creating copies. Another small improvement on top of that would be to convert str to a char[] using str.toCharArray() and use array indexes to access elements of the string.-
s1.equals(new StringBuilder(s1).reverse().toString()) is a lazy way to check if a string is a palindrome. It creates a new string, and then iterates over values to check if palindrome. It would be a lot better to implement a function for that, comparing pairs of letters from the beginning and end, closing in toward the center.-
!palindromeArray.contains(s1) is an \$O(n)\$ operation that can be easily turned into \$O(1)\$ by using a Set instead of an array.-
The slowest part of all is the main algorithm of enumerating possible palindromes. The nested loop basically checks all possible substring. That's not necessary. A more clever approach would be, for each position, center on the position, and keep going outward as long as the start and end letters still form a palindrome. This way you can drastically reduce the number of comparisons made in the process of checking for palindromes.
Coding style
The coding style of your post is extremely poor.
- Indentation is inconsistent, messy
- Names are poor.
palindromeArrayis not an "array", it's a list. And you could just call itpalindromes, naturally.subPalis cryptic at best. How aboutprintUniquePalindromes?
- It's recommended to use braces with single-statement
ifandforstatements too.
Other issues
The
scan != null condition is always true, so the if statement is unnecessary.The initializer in
String s1 = "" is redundant, String s1 would be enoughContext
StackExchange Code Review Q#146142, answer score: 2
Revisions (0)
No revisions yet.