0

When executing my program it keeps displaying "null" on line 13 I want to know what's wrong on my algorithm for it keeps printing null.

private class SpadeIterator implements Iterator<Card>{
    private int nextCardSpade;
    private List<Card> cards;
    private int count=0;
    private SpadeIterator(List cards) {
        this.cards=cards;
        this.nextCardSpade = cards.size()-1;
    }

    @Override
    public boolean hasNext() {
        count++;
        if(nextCardSpade<0)
            return false;
        //nextCardSpade--;
        return true;
    }

   @Override
    public Card next() {

        int i=0;
        this.count=i;
        Card temp = cards.get(nextCardSpade);

        while(hasNext()){    //find SPADES
            temp=cards.get(nextCardSpade--);
            i++;

            if(temp.suit.value == Suit.SPADES.value)
                return temp;
        }
        //DONT MOVE
        return null;
        //nextCardSpade--;      //DONT DELETE

    }
}

Current Results

The results are meant to show the 13 spades without returning null at the end.

ankuranurag2
  • 2,300
  • 15
  • 30
  • Then just check for the null – Ravi Jan 20 '19 at 14:55
  • Your `next()` method shouldn't contain any case which returns `null`. If there is no proper element to be returned then it is `hasNext()` method job to return `false`. – Pshemo Jan 20 '19 at 15:01
  • 1
    Also your `hasNext` method should not modify the state (increasing count) of the object in any way. – Joakim Danielson Jan 20 '19 at 15:04
  • Yeah but i need to return something otherwise the program doesn't run –  Jan 20 '19 at 15:11
  • If there is no next then ```next``` is supposed to throw an NoSuchElementException, not return a value. https://docs.oracle.com/javase/7/docs/api/java/util/Iterator.html#next(). And most callers will refrain from calling ```next``` unless ```hasNext``` says there is a next element. –  Jan 20 '19 at 17:42

2 Answers2

0

Check that nextCardSpade equals to 0 also:

if (nextCardSpade <= 0)
Ori Marko
  • 56,308
  • 23
  • 131
  • 233
0

Your next() method shouldn't contain any case which would return invalid value like null. If there is no next element to be returned it is hasNext() method's job to return false to prevent you from calling next().

So your code should look more like

class SpadeIterator implements Iterator<Card>{
    private int spadesCounter = 0;
    private Iterator<Card> cardsIt;

    private SpadeIterator(List<Card> cards) {
        cardsIt = cards.iterator();
    }

    @Override
    public boolean hasNext() {
        return spadesCounter<13; // we can't put spacesCounter++ here because 
                                 // we should be able to call `hasNext()` many times
                                 // and still get same answer,
                                 // so `hasNext()` shouldn't change any state 
                                 // (at least one which could cause changing its result)
    }

    @Override
    public Card next() {
        Card temp = cardsIt.next(); //if our `hasNext()` returned `false` but we 
                                    //didn't check or ignored it, this will CORRECTLY 
                                    //throw NoSuchElementException 
        while(temp.suit.value != Suit.SPADES.value){
            temp = cardsIt.next();
        }
        spadesCounter++;
        return temp;
    }
}

OR if you simply want to iterate over list and print only selected elements you can use streams with filtering like

List<Card> cards = ...//not really important how get it
cards.stream()
     .filter(card -> card.suit.value == Suit.SPADES.value)
     .forEach(card -> System.out.println(card));

or even simpler

for (Card card : cards){
    if(card.suit.value == Suit.SPADES.value){
        System.out.println(card);
    }
}
Pshemo
  • 122,468
  • 25
  • 185
  • 269
  • 1
    There's a requirement to throw NoSuchElementException when there is no next element to return from ```next()```. In your code this is implicit in the call to ```cardsIt.next()```. You likely know this, I'm just pointing it out for the original poster. –  Jan 20 '19 at 17:46
  • @another-dave Thanks for pointing that out. I updated my answer a little to include this info. – Pshemo Jan 20 '19 at 18:18