patternjavaMinor
Is my N-drome (variation of palindrome) checking program efficient?
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
abcdabsince its 2-character units areab,cd, andab, 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
-
What is the purpose of
What about strings
-
The lack of an increment value in the
-
I'm not familiar with
-
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.