1

I'm trying to write a Java method to preform a "multi-pop" off a stack.

It should perform a "pop" operation on a stack object k number of times. This is what I'm thinking, but it's not quite right. Any help out there?

public void multipop(int k) {
    while (top != null) {
        for (int i = 0; i <= k; i++) {
            this.pop();
        }
    }
}
andr
  • 15,970
  • 10
  • 45
  • 59
Benzle
  • 373
  • 3
  • 9
  • 18

5 Answers5

6
  1. You execute the while loop until the stack is exhausted, which is probably not what you want. If you want to check whether there are elements in the stack, use an if statement.
  2. In the loop, you iterate from 0 to k, inclusive. This means that if k = 3, you go through 0, 1, 2 and 3 and thus call this.pop() four times.
  3. Even if you replace the while with an if, you only check that there is one element is on the stack, but you may call pop() multiple times. You should do the check inside the loop or move the check inside pop().
  4. The indentation is horrible :)
andri
  • 11,171
  • 2
  • 38
  • 49
3

There are a few issues with this:

  1. The brackets should be better formatted [the first look lead me to believe a mismatch]
  2. Your check for the null case should be in the middle of the for loop: for (... ; i<=k && stack.canPop(); ...
  3. You need a method to check to make sure that there is an item that you can pop.
  4. As the other answer states, there is an off by one error, if you wish to pop up to K items, then the condiction should be i < k.

This should run into an exception or an infinite loop because the first loop makes sure that there is still a "top" variable that isn't null, and then it directs it to the second loop that goes from 0:k.

monksy
  • 14,156
  • 17
  • 75
  • 124
3

Looks like an off-by-one error.

If k=1, you will go through the loop with i=0 and i=1. You can fix this by changing i<=k to i<k

Mercurybullet
  • 889
  • 1
  • 10
  • 20
  • Although I appreciate the recognition, I will admit that some of the other answers address important issues that my answer does not. For example, that while loop will continue running the inner for loop until the stack is empty (in other words, until top==null). As suggested by others, this can be fixed by changing that while to an if and moving it inside the for loop (swapping positions). – Mercurybullet Nov 19 '09 at 00:44
0

You're off by one. You want 

 for(int i =0; i < k; i++)

If there's more problems, you need to provide more code and questions.

nos
  • 223,662
  • 58
  • 417
  • 506
0

1st, it loops (k+1) times, from 0 to k. 2nd, after several pops, it is possible that top is null. So it needs check top all the time.

It could be modified as below:

public void multipop(int k) {

    for (int i = 0; top != null && i < k; i++) {
        this.pop();
    }

}

Steel
  • 125
  • 2
  • 7