tags:

views:

164

answers:

4

Hey y'all,

I'm trying to write a Java method to preform a 'multi-pop" on a stack, it should "pop" "k" number of items off the top of the stack object. This is what I'm thinking, but it's not quite right, any help out there?

Thanks everyone.

public void multipop(int k){

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

}}}
+2  A: 

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
+1  A: 

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
Gets my vote for being most concise best answer! Thanks
Benzle
That is not the only error that is in the code.
monksy
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
A: 

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
+5  A: 
  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