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

Outputs all substrings that are palindromes in a given string, in alphabetical order without repitition

Submitted by: @import:stackexchange-codereview··
0
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:

-
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. palindromeArray is not an "array", it's a list. And you could just call it palindromes, naturally. subPal is cryptic at best. How about printUniquePalindromes?



  • It's recommended to use braces with single-statement if and for statements 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 enough

Context

StackExchange Code Review Q#146142, answer score: 2

Revisions (0)

No revisions yet.