views:

259

answers:

5

This is a stylistic question. I want to loop twice with a variable on which is set to false, then to true. Which of these is clearer:

A)

for (final boolean on : new boolean[] { false, true} )
{
   doStuffBasedOnABooleanFlag(on);
}

B)

for (int i = 0; i < 2; ++i)
{
   final boolean on = (i == 1);
   doStuffBasedOnABooleanFlag(on);
}

C) something else


edit: Murphy's law of unintended interpretations comes into play... the use case I have originally looked something like this instead of doStuffBasedOnABooleanFlag:

for (final boolean on : new boolean[] { false, true} )
{
   JButton button = on ? onButton : offButton;
   button.addActionListener(new ActionListener() {
      @Override public void actionPerformed(ActionEvent event) {
      doStuffLaterBasedOnABooleanFlag(on);
      }
   }
}

But I think I like Brendan's answer, I'll just refactor the loop contents into a separate method:

doStuffBasedOnABooleanFlag(false);
doStuffBasedOnABooleanFlag(true);

   ...

private void doStuffBasedOnABooleanFlag(final boolean on)
{
   JButton button = on ? onButton : offButton;
   button.addActionListener(new ActionListener() {
      @Override public void actionPerformed(ActionEvent event) {
      doStuffLaterBasedOnABooleanFlag(on);
      }
   }
}
+14  A: 

Since it's two lines, I'd just skip the loop and do:

doStuffBasedOnABooleanFlag(false);
doStuffBasedOnABooleanFlag(true);

Less code, more obvious, more efficient.

Brendan Long
I prefer this answer. Simple, clear, and impossible to misinterpret.
Joshua
i like that. the for, final, i = 0 while <2 with i == 1?!? and whatever is way too complicated. just call the method twice! :) i had to delete my answer because i've got it wrong from looking at the question too loosely. two simple method calls are much easier to read.
stmax
btw you've got the true/false in the wrong order, too ;)
stmax
+1. A lot more clear.
mheathershaw
I got the order wrong at first but it's fixed (false then true). Thanks stmax.
Brendan Long
I like this too. My real code didn't have two method calls, it had about 5 lines of stuff including an inner anonymous class, which I didn't want to repeat twice. For some reason I was missing the obvious (?) refactoring into a method call, and using the do-it-in-a-loop approach instead.
Jason S
+2  A: 

If you really want to use a loop, I would go with (a). While it's novel, it's also clear and efficient. I might move the boolean array to a private static, to avoid recreating the array every time.

But I like Brendan's answer better.

Software Monkey
+6  A: 

Another option would be to avoid the boolean and use an enum:

enum Mode { APPEND, REPLACE } // or whatever your boolean indicated

You could then either iterate:

for(Mode m : Mode.values()) doStuff(m);

Or do the calls directly:

doStuff(Mode.APPEND);
doStuff(Mode.REPLACE);

The advantage of this would be that the API indicates more clearly what's happening.

Fabian Steeg
Awesome. You might even end up putting the code that WAS in your loop into your Mode class itself, Mode.APPEND.doStuff(). Otherwise doStuff is a non-OO utility method--yuck.
Bill K
@Bill K: I agree, but doStuff() in my case is a non-static method that needs access to other non-static methods.
Jason S
Enums methods can be non-static, and have access to other items inside the enum. They are very powerful, in fact. enums are full-featured classes, just limited to pre-defined instances.
Bill K
A: 

(er, someone please delete this.

sorry about that!)

Java Expert
You can delete your own answers by clicking the "delete" button (next to edit)
Brendan Long
A: 

It's not just the loop, I'm also really uncomfortable with the use of booleans in this way.

What about something like:

  ActionListener myListener = new ActionListener() {
    @Override
    public void actionPerformed(ActionEvent event) {
      doStuffLaterBasedOnABooleanFlag(event.getSource() == onButton);
    }
  };
  onButton.addActionListener(myListener);
  offButton.addActionListener(myListener);

That still leaves the boolean within the listener, but without knowing what the doStuffLater method does that's as far as we can go.

CurtainDog
I need the boolean. I can't post my real code, it's more complicated than this, and I have two lists of UI components, one handled one way, the other handled the other way.
Jason S