views:

267

answers:

10

I have a set of objects I'd like to do some operations on, in the order they're iterated. After that operation gets called on them, I'd like to perform other operations on them. Basically, the code will look sort of like this:

for(int i = 0;i < myobj.size();i++)
{
   myobj.at(i).doSomething();
}

for(int i = 0;i < myobj.size();i++)
{
   myobj.at(i).doSomethingElse();
}

This looks kind of ugly to me. How could I rewrite this into something better? The order of the operations should stay the same.

A: 

ummm, would this work for you?

for(int i = 0;i < myobj.size();i++)
{   
    myobj.at(i).doSomething();
    myobj.at(i).doSomethingElse();
}
Jaimal Chohan
He said the order of the operations has to be the same.
Jack Leow
This changes the order -- A1B2C3 instead of ABC123.
a paid nerd
+5  A: 

Use a foreach loop:

for (MyObject currentObject : myobj) {
  currentObject.doSomething();
}

for (MyObject currentObject : myobj) {
  currentObject.doSomethingElse();
}
ChssPly76
+22  A: 

I don't know what myobj is but if it's Iterable, then you could use a foreach loop:

for (Foo foo : myobj) {
  foo.doSomething();
}

for (Foo foo : myobj) {
  foo.doSomethingElse();
}

If it's not Iterable, then making it so might help other code too.

daveb
+1 for the "if it's `Iterable`"
Pascal Thivent
+1 for the same.
+1 for the most readable solution. Btw, the higher up abstraction will probably doo `FooCollection coll = ...; coll.doSomethingAndSomethingElse()` to encapsulate both loops.
mhaller
A: 

You could always do something equally as ugly like

bool d=false;
for(int i = 0;i < myobj.size();i++)
{
   if(d==false){
     myobj.at(i).doSomething();
   }else{
     myobj.at(i).doSomethingElse();
   }
   if(i==myobj.size()-1 && d==false){ d=true; i=0;}
}
Earlz
I wouldn't call this "equally" ugly.
Jorn
That's not equally as ugly - it's far worse.
adrianbanks
Well I was going to put in a goto, but Java doesn't have that does it? rofl
Earlz
+6  A: 

If you absolutely must have both operations in different loops, refactor the loops out into appropriately named methods so that it's easier to understand at a glance:

createData();
saveData();
Ross Martin
+4  A: 

This looks like an excellent opportunity for you to apply the Visitor pattern.

Daniel Pryden
A: 

Slightly different from other offerings: have your calls to the methods return the next item to operate on, with a well defined terminate return (e.g., null):

while ((holder = holder.doSomething())!=null);
while ((holder = holder.doSomethingElse())!=null);

With some initial and intervening re-assignment of holder, obviously. Thumbs up on the Visitor suggestion, implementing Iterable is also a nice option; my solution could also be viewed as implementing Iterator with myObj:

while (myObj.hasNext()) myObj.next().doSomething();
myObj.resetIterator();
while (myObj.hasNext()) myObj.next().doSomethingElse();

One downside is if you ever get to the point that doSomething() and doSomethingElse() can be applied in a single loop, instead of two separate ones, you have to have a merged method to make this work and take advantage of that change.

An alternative on the for-loop, using a post-fix (actually inspired by that awful, obfuscating solution):

int size = myObj.size(), i=0;
while (i<size) myObj.at(i++).doSomething();
i=0;
while (i<size) myObj.at(i++).doSomethingElse();

and, if only the order of doSomething() on all and doSomethingElse() on all matters (i.e., not which index order they are visited in), you could even skip the reassignment there in the middle, and just pre-fix decrement for the second call.

int size = myObj.size(), i=0;
while (i<size) myObj.at(i++).doSomething();
while (i>0) myObj.at(--i).doSomethingElse();

which of course allows for an even sexier version of the Iterator solution above if myObj implements ListIterator instead of Iterator (okay, I'll stop editing more stuff in now...seriously).

Carl
A: 

This is an alternative way of writing the same.

I think, it is the Visitor pattern, although all the times I have seen it I don't quite get it ( the Visitor documentation ) so, probably this is something else.

Anyway, the idea is to use anonymous inner classes ( fake Java "closures" ) and write the iteration once.

public class IterationSample {
    public static void main( String [] args ) {

        Foo [] items = new Foo[0]; // get it from somewhere  ....

        iterate( items , new _(){void with( Foo f ){
            f.doSomething();
        }});    

        iterate( items , new _(){void with( Foo f ){
            f.doSomethingElse();
        }});    

        iterate( items , new _(){void with( Foo f ){
            f.doBar();
        }});   



    }
    // write  the loop once.
    static void iterate( Foo [] items, _ visitor ) {
        for( Foo f : items ) {
            visitor.with( f );
        }
    }
}

The Foo object would be the object would be your object.

// Not really abstract just for the sake of the sample. Use your own.
abstract class Foo {
    abstract void doSomething();
    abstract void doSomethingElse();
    abstract void doBar();

}

Finally this is the "visitor"

// made abstract instead of interface just to avoid having to type 
// "public" each time. 
abstract class  _ {
    abstract void with( Foo f );
}

It is a shame I can't avoid the return type of the method.

Otherwise this

         iterate( items ,new _(){ with( Foo f ){

Would read as:

Iterate items "noise" with foo

Why would you do that when you can do this?

Because that way you can have an array of operations:

 _ [] operations = {
        new _(){void with( Foo f ){
            f.doSomething();
        }},
        new _(){void with( Foo f ){
            f.doSomethingElse();
        }},
        new _(){void with( Foo f ){
            f.doBar();
        }}
 };

And iterate them :)

  for( _ andPerformAction : operations ) {
      iterate( items , andPerformAction );
  }

There might be some situations for this, although I can think of any right now =-S

OscarRyz
+3  A: 

http://www.iam.unibe.ch/~akuhn/blog/2008/pimp-my-foreach-loop/

cwash
Whoa... that's nice but.. I don't get it :( ... I will take a look at it with a fresher mind. :)
OscarRyz
If you pull in Hamcrest you may be able to use these as well: http://code.google.com/p/hamcrest-collections/wiki/GettingStarted
cwash
A: 

By using lambdaj you can obtain the same result without writing any explicit loop as it follows:

forEach(myobj).doSomething();
forEach(myobj).doSomethingElse();
Mario Fusco