4

Full disclosure: this is for an assignment, so please don't post actual code solutions!

I have an assignment that requires me to take a string from the user and pass it into a stack and a queue, then use those two to compare the chars to determine if the string is a palindrome. I have the program written, but there appears to be some logic error somewhere. Here's the relevant code:

public static void main(String[] args) {

    UserInterface ui = new UserInterface();
    Stack stack = new Stack();
    Queue queue = new Queue();
    String cleaned = new String();
    boolean palindrome = true;

    ui.setString("Please give me a palindrome.");
    cleaned = ui.cleanString(ui.getString());

    for (int i = 0; i < cleaned.length(); ++i) {
        stack.push(cleaned.charAt(i));
        queue.enqueue(cleaned.charAt(i));
    }

    while (!stack.isEmpty() && !queue.isEmpty()) {
        if (stack.pop() != queue.dequeue()) {
            palindrome = false;
        }
    }

    if (palindrome) {
        System.out.printf("%s is a palindrome!", ui.getString());
    } else
        System.out.printf("%s is not a palindrome :(", ui.getString());

    stack.dump();
    queue.clear();

}

 public class Stack {

   public void push(char c) {
    c = Character.toUpperCase(c);
    Node oldNode = header;
    header = new Node();
    header.setData(c);
    header.setNext(oldNode);
  }

  public char pop() {
    Node temp = new Node();
    char data;
    if (isEmpty()) {
        System.out.printf("Stack Underflow (pop)\n");
        System.exit(0);
    }
    temp = header;
    data = temp.getData();
    header = header.getNext();
    return data;
  }

}

public class Queue {

  public void enqueue(char c) {
    c = Character.toUpperCase(c);
    Node n = last;
    last = new Node();
    last.setData(c);
    last.setNext(null);
    if (isEmpty()) {
        first = last;
    } else n.setNext(last);     
  }

  public char dequeue() {
    char data;
    data = first.getData();
    first = first.getNext();
    return data;
  }

}

public String cleanString(String s) {
    return s.replaceAll("[^A-Za-z0-9]", "");
}

Basically, when running my code through the debugger in Eclipse, my pop and dequeue methods appear to only select certain alphanumerics. I am using replaceAll("[^A-Za-z0-9]", "") to "clean" the user's string of any nonalphanumeric chars (!, ?, &, etc.). When I say it only selects certain chars, there doesn't seem to be any pattern that I can discern. Any ideas?

Servy
  • 202,030
  • 26
  • 332
  • 449
idigyourpast
  • 714
  • 4
  • 12
  • 24
  • 2
    Can you give an example of an input that would cause the program to fail? I don't see anything immediately wrong with your algorithm. – tskuzzy Jun 08 '12 at 16:16
  • Perhaps this is a silly question on my part, but why use ++i instead of i++ in this case? – BlackVegetable Jun 08 '12 at 16:18
  • @tskuzzy I've been using a few, but usually "Too hot to hoot". I'm also setting all chars to uppercase, so there shouldn't be any problems caused by case. – idigyourpast Jun 08 '12 at 16:20
  • With some compilers/languages, the generated code might actually be more efficient for ++i. I don't think it affects Java though. (I don't remember the actual reason why it could be made faster -- something to do with registers?) – Marvo Jun 08 '12 at 16:21
  • @BlackVegetable It's just a style preference. A very particular professor turned me on to it. Supposed to be faster in high-performance situations, though when using Java I suppose that doesn't matter much. – idigyourpast Jun 08 '12 at 16:22
  • @Marvo i++ returns the value of i, iterates it, then returns the iterated value. ++i simply returns the iterated value without the additional steps. I can't imagine this makes much of a difference unless you're iterating millions of steps. – idigyourpast Jun 08 '12 at 16:24
  • Can you post the code for cleanString? I realize it is that regex manipulation but is it only that one line of code? – BlackVegetable Jun 08 '12 at 16:30
  • Your `pop()` method seems to have a lot of unnecessary statements. Could you also show the inplementation of `push()`, `enqueue()` and `cleanString()`? – rsp Jun 08 '12 at 16:31
  • It is better to edit your post to include more code, instead of adding them as comments. – rsp Jun 08 '12 at 16:32
  • @rsp added the requested code – idigyourpast Jun 08 '12 at 16:47
  • Why don't you use Java's default implementation of a Stack and a Queue? – Lai Xin Chu Jun 08 '12 at 16:51
  • Certain alphanumerics as in: only uppercase? (you convert all to uppercase in both `push()` and `enqueue()`) – rsp Jun 08 '12 at 16:52
  • @LaiXinChu The assignment requires that we implement our own versions of Stack and Queue. – idigyourpast Jun 08 '12 at 17:55
  • @rsp I've already got .toUpperCase() in both push and enqueue, so that isn't causing the problem. Honestly it almost seems as if items are being popped or dequeued before the call is being made. – idigyourpast Jun 08 '12 at 17:57
  • The use of the Stack class in java should be avoided unless you really need it. It extends Vector, which adds a lot of overhead due to synchronization (so that it's thread safe). You're better off with a Deque which, while it is not a clean stack interface, allows you to treat it as a stack with push and pop methods (this is even what the javadocs for Stack recommend using instead). PS: Remove the System.exit, as it's bad form. Throw an IllegalStateException instead. – Matt Jun 08 '12 at 18:07
  • I meant: because you use `toUppercase()` it is to be expected you never get lowercase characters from your input string. Could you provide a test input string, add some debugging and show us the results of a testrun? – rsp Jun 08 '12 at 18:45

2 Answers2

0

Your general algorithm works properly, assuming your queue and stack are correct (i tried this using the Deque implementations found in the jdk). Since your assignment involves the datastructures, i've pretty much just took your main logic and replaced the datastructures with ArrayDequeue, so I don't feel like i'm answering this for you.

    String word = "ooffoo";

    word = word.replaceAll("[^A-Za-z0-9]", "");

    Deque<Character> stack = new ArrayDeque<Character>(word.length());
    Deque<Character> queue = new ArrayDeque<Character>(word.length());

    for (char c : word.toCharArray()) {
        stack.push(c);
        queue.add(c);
    }

    boolean pal = true;

    while (! stack.isEmpty() && pal == true) {
        if (! stack.pop().equals(queue.remove())) {
            pal = false;
        }
    }

    System.out.println(pal);
Matt
  • 11,523
  • 2
  • 23
  • 33
0

I'd recommend using a debugger to see exactly what was being compared, or at the very least spit out some print lines:

while (!stack.isEmpty() && !queue.isEmpty()) {
    Character sc = stack.pop();
    Character qc = queue.dequeue();
    System.out.println(sc + ":" + qc);
    if (sc != qc) {
        palindrome = false;
    }
}
KidTempo
  • 910
  • 1
  • 6
  • 22
  • Also, make a habit of unit testing all your classes. If your stack and queue implementations work as expected, you are less likely to mess them up later on when working on something else and go back to make "just a little tweak". – KidTempo Jun 08 '12 at 18:22
  • when used as a loop incrementer, ++i and i++ are exactly the same. it only matters when the actual returned value is used. In this case, whether you use i++ or ++i, the result is the same, since no one is using the return value. – Matt Jun 08 '12 at 18:50
  • Yeah, I guess that makes sense now that I think about it. – KidTempo Jun 08 '12 at 20:09