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

Is my N-drome (variation of palindrome) checking program efficient?

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

Problem


  • An N-Drome is a string that is the same forwards and backwards when split into n-character units.



  • For example, a string would be a 1-drome if and only if it were a standard palindrome.



  • An example of a 2-drome would be abcdab since its 2-character units are ab, cd, and ab, and its first and last 2-character units are identical.



package ndrome;

import java.util.Scanner;

public class NDrome {
    public static void main(String[] args) {
        System.out.print("enter a string: ");
        Scanner in = new Scanner(System.in);
        String original = in.nextLine();
        System.out.print("enter the n value: ");
        int n = in.nextInt();
        if(n-1>=original.length()){
            System.out.println("Error:N value is greater than string length! exiting...");
            System.exit(0);
        }
        int y = 2;
        here: for (int i = 0, j = original.length() - 1;;) {
            int x = n - 1;
            String a = "", b = "";
            while (x >= 0) {
                if (i >= original.length()) {
                    break here;
                } else {
                    a = a + original.charAt(i);
                    i++;
                    b = b + original.charAt(j - x);
                    x--;
                }
            }
            j=j-n;
            if (a.equals(b)) {
                y = 1;
            } else {
                y = 0;
                break;
            }
        }
        if (y == 1) {
            System.out.print("this is a " + n + "-drome");
        } else if (y == 0) {
            System.out.print("this is not a " + n + "-drome");
        }
    }
}

Solution

I'm not a Java programmer, but I'll point out some familiar syntactical things I see:

-
You should separate the logic by putting the checking code into a separate function, leaving main() to call the function, handle the input/output, and terminate when needed.

-
What is the purpose of y? From the looks of it, it's used to hold an "error condition" for indicating whether or not the inputted string is an N-drome. If so, I think this should be a boolean type and given a relevant name. Either way, y should be renamed to reflect its purpose.

What about strings a and b? Are they the leftmost one and the rightmost one for the original string? You should write out the purpose of these single-char variables to give them more meaning.

-
The lack of an increment value in the for-loop doesn't look too clear. If someone wanted to see what the loop should do after each iteration, they would have to fish through the loop body. If the loop won't be incremented in each control path, then it may not be the best loop type to use.

-
I'm not familiar with break here, but it looks like a goto in C++. I don't think something like that should be done with a loop (or probably anything practical). Again, you may need to rethink how you approach this loop type and execution.

Context

StackExchange Code Review Q#41840, answer score: 5

Revisions (0)

No revisions yet.