Implement Java Iterator and Iterable in same class?

Question

I am trying to understand Java Iterator and Iterable interfaces

I am writing this class

class MyClass implements Iterable<String> {
    public String[] a = null;
    public MyClass(String[] arr) {
        a = arr;    
    }

    public MyClassIterator iterator() {
        return new MyClassIterator(this);
    }

    public class MyClassIterator implements Iterator<String> {
        private MyClass myclass = null;
        private int count = 0;
        public MyClassIterator(MyClass m) {
            myclass = m;    
        }

        public boolean hasNext() {
            return count < myclass.a.length;
        }
        public String next() {
            int t = count;
            count++;
            return myclass.a[t];
        }

        public void remove() {
            throw new UnsupportedOperationException();
        }
    }   
}

It seems to be working.

Should I have:

Myclass implements Iterable<Stirng>, Iterator<String> {

}

Or I should put MyClassIterator outside MyClass as

class MyClass implements Iterable<String> {
    public String[] a = null;
    public MyClass(String[] arr) {
        a = arr;    
    }
    public MyClassIterator iterator() {
        return new MyClassIterator(this);
    }
}


    public class MyClassIterator implements Iterator<String> {
        private MyClass myclass = null;
        private int count = 0;
        public MyClassIterator(MyClass m) {
            myclass = m;    
        }

        public boolean hasNext() {
            return count < myclass.a.length;
        }
        public String next() {
            int t = count;
            count++;
            return myclass.a[t];
        }

        public void remove() {
            throw new UnsupportedOperationException();
        }
    }

Which one is better?

Why does your class need to do both? In general you would implement Iterable so someone can get an Iterator from your class. — Kal, Apr 29 '11 at 19:01

score 35 · Accepted Answer · answered Apr 29 '11 at 19:00

35

You should almost never implement both Iterable and Iterator in the same class. They do different things. An iterator is naturally stateful - as you iterate using it, it has to update its view of the world. An iterable, however, only needs to be able to create new iterators. In particular, you could have several iterators working over the same original iterable at the same time.

Your current approach is pretty much okay - there are aspects of the implementation I'd change, but it's fine in terms of the separation of responsibilities.

answered Apr 29 '11 at 19:00

Jon Skeet

1,421,763
867
9,128
9,194

thanks, i updated a bit my question, which one is better, how about put MyClassInterator outside MyClass? – icn Apr 29 '11 at 19:01
2

@icn: Keeping MyClassIterator as a nested class is fine. (Iterator though, not Interator.) I suggest you make it a private static final class though. You don't need to be able to refer to it directly from anywhere else, after all. – Jon Skeet Apr 29 '11 at 19:03
@icn: You should keep `MyClassIterator` inside `MyClass`. You should also make it `private` and make the method return `Iterator`. You should also not pass in `MyClass` to the constructor because `MyClassIterator` is an inner class and has an implicit reference to the surrounding instance of `MyClass`, so you can just access the array `a` directly in it. – ColinD Apr 29 '11 at 19:04
2

@Jon - why would you make the inner class static? – Stephen Swensen Apr 29 '11 at 19:10
@Stephen: you're not using the implicit reference to an instance of the outer class, so why *not* make it static? – Jon Skeet Apr 29 '11 at 19:41
1

@Jon: `Iterator`s are often (including in this case) perfect for non-static inner classes though. He's not using the implicit reference to the outer class but... he should be! =) – ColinD Apr 29 '11 at 19:47
@Jon - ah, in general I'd certainly agree with that philosophy, but in this case I think it makes sense to instead get rid of the constructor which takes `MyClass` and go ahead and use the implicit reference. – Stephen Swensen Apr 29 '11 at 19:51
@Stephen: that would certainly work too. Personally I'm not a fan of the implicit reference to start with. Awkward syntax, and too easy to forget it's there. Just my personal preference though :) – Jon Skeet Apr 30 '11 at 13:01

Stephen Swensen · Answer 2 · 2011-04-29T20:00:50.433

You were on track with your first try. MyClass only needs to implement Iterable<String>, which in turn requires you to provide an Iterator<String> implementation to return from Iterable<String>.iterator().

There's no need to put the MyClassIterator outside of MyClass because in most cases you will never even need to directly use the Iterator<String> (it's used implicitly by the for .. in .. syntax on Iterable<String>s), and in all other cases the interface is sufficient unless you actually add additional behavior to the implementation (which you likely won't ever need to do).

Here's how I'd do it, see comments inlined:

import java.util.Iterator;

class MyClass implements Iterable<String>{
    public String[] a=null; //make this final if you can
    public MyClass(String[] arr){
        a=arr; //maybe you should copy this array, for fear of external modification
    }

    //the interface is sufficient here, the outside world doesn't need to know
    //about your concrete implementation.
    public Iterator<String> iterator(){
        //no point implementing a whole class for something only used once
        return new Iterator<String>() {
            private int count=0;
            //no need to have constructor which takes MyClass, (non-static) inner class has access to instance members
            public boolean hasNext(){
                //simplify
                return count < a.length;
            }
            public String next(){
                return a[count++]; //getting clever
            }

            public void remove(){
                throw new UnsupportedOperationException();
            }
        };
    }
}

Thanks, since we have a exception "throw new UnsupportedOperationException();", somewhere in the program we should specify that an excepriton to be throwed something like throw UnsupportedOperationException()? — icn, Apr 29 '11 at 21:09
Your welcome, @icn. Yes, it's probably sufficient to just mention it in the comments for `iterator()`. In practice it won't matter, `for .. in ..` is pretty much the only construct which makes direct use of `Iterator`s in (modern) Java and it doesn't attempt to use `remove`. Indeed, "optional" interface members such as `remove` are useless at best since consumers pretty much have to assume they will throw. — Stephen Swensen, Apr 29 '11 at 22:03

score 2 · Answer 3 · answered Apr 29 '11 at 19:01

You should not do Myclass implements Iterable<String>,Iterator<String>{ since iterators are single-use. With the exception of list iterators, there's no way to return them to the start.

Incidentally, you can skip the

MyClass myClass;
public MyClassInterator(MyClass m){
  myclass=m;  
}

and instead of referencing

myClass

reference

MyClass.this

Your inner class is not static, so MyClass.this will reference the instance of the enclosing class that created it.

Implement Java Iterator and Iterable in same class?

3 Answers3

Linked