11

After my array in the for loop reaches the last index, I get an exception saying that the index is out of bounds. What I wanted is for it to go back to the first index until z is equal to ctr. How can I do that?

My code:

char res;
int ctr = 10
char[] flames = {'F','L','A','M','E','S'};

for(int z = 0; z < ctr-1; z++){
    res = (flames[z]);
    jLabel1.setText(String.valueOf(res));
}
blackpanther
  • 10,998
  • 11
  • 48
  • 78
pchan
  • 115
  • 1
  • 1
  • 9
  • use `%` while accessing `flames[]` like...`flames[z%flames.length()];` – boxed__l Sep 10 '13 at 17:37
  • use the mod (%) operator on the index expression : `res = flames[z % flames.length()];` Or better yet, have an *invariant* variable `n = flames.length()` just outside of the for-loop, and then have `res = flames[z % n];` – luis.espinal Sep 10 '13 at 17:50
  • 2
    @luis.espinal : Won't the compiler do that for us? `flames.length()` in a loop would be optimized by the compiler right/shouldn't it ? – boxed__l Sep 10 '13 at 18:19
  • @boxed__l - good question. I don't believe it does IIRC. One way to confirm this is to use the javap disassembler on both versions (with and w/o the invariant out of the loop) and compare the output of both. – luis.espinal Sep 12 '13 at 13:46
  • @luis.espinal : Seems you are right. Did disassembling on a similar code. No optimization. There are projects like [ProGuard](http://proguard.sourceforge.net/) available to do this optimization and much more. +1 – boxed__l Sep 13 '13 at 17:24
  • I wouldn't rely on products like that to optimize such things, though. Invariants are invariants, and code that needlessly recomputes an invariant is bad code. An optimizer that "fixes" that kind of bad practice simply swipes the garbage under the rug. The proper use of an optimizer is for detecting, and improving other things, like tail-call optimization and such. – luis.espinal Sep 13 '13 at 20:03
  • Also, even with compilers with optimizers, say gnu gcc, it is nearly impossible for it to know if a function call or complex expression is truly **invariant**. In the case of a function call (in this case `flames.length()`), the language would have to provide some type of syntactic support (.ie. a keyword or modifier that says a function is truly const, immutable, cacheable or idempotent) for the compiler to make the determination to move its reference out of a loop. Java, the language, certainly does not have that kind of syntactic thing. – luis.espinal Sep 13 '13 at 20:08

5 Answers5

7

You need to use an index that is limited to the size of the array. More precisely, and esoterically speaking, you need to map the for-loop iterations {0..9} to the valid indexes for the flame array {0..flames.length()-1}, which are the same, in this case, to {0..5}.

When the loop iterates from 0 to 5, the mapping is trivial. When the loop iterates a 6th time, then you need to map it back to array index 0, when it iterates to the 7th time, you map it to array index 1, and so on.

== Naïve Way ==

for(int z = 0, j = 0; z < ctr-1; z++, j++)
{
      if ( j >= flames.length() )
      {
         j = 0; // reset back to the beginning
      }
      res = (flames[j]);
      jLabel1.setText(String.valueOf(res));
}

== A More Appropriate Way ==

Then you can refine this by realizing flames.length() is an invariant, which you move out of a for-loop.

final int n = flames.length();
for(int z = 0, j = 0; z < ctr-1; z++, j++)
{
      if ( j >= n )
      {
         j = 0; // reset back to the beginning
      }
      res = (flames[j]);
      jLabel1.setText(String.valueOf(res));
}

== How To Do It ==

Now, if you are paying attention, you can see we are simply doing modular arithmetic on the index. So, if we use the modular (%) operator, we can simplify your code:

final int n = flames.length();
for(int z = 0; z < ctr-1; z++)
{
      res = (flames[z % n]);
      jLabel1.setText(String.valueOf(res));
}

When working with problems like this, think about function mappings, from a Domain (in this case, for loop iterations) to a Range (valid array indices).

More importantly, work it out on paper before you even begin to code. That will take you a long way towards solving these type of elemental problems.

luis.espinal
  • 10,331
  • 6
  • 39
  • 55
7

While luis.espinal answer, performance-wise, is better I think you should also take a look into Iterator's as they will give you greater flexibility reading back-and-forth.

Meaning that you could just as easy write FLAMESFLAMES as FLAMESSEMALF, etc...

int ctr = 10;
List<Character> flames = Arrays.asList('F','L','A','M','E','S');
Iterator it = flames.iterator();

for(int z=0; z<ctr-1; z++) {
    if(!it.hasNext()) // if you are at the end of the list reset iterator
        it = flames.iterator();

    System.out.println(it.next().toString()); // use the element
}

Out of curiosity doing this loop 1M times (avg result from 100 samples) takes:

               using modulo: 51ms
            using iterators: 95ms
using guava cycle iterators: 453ms

Edit: Cycle iterators, as lbalazscs nicely put it, are even more elegant. They come at a price, and Guava implementation is 4 times slower. You could roll your own implementation, tough.

// guava example of cycle iterators
Iterator<Character> iterator = Iterators.cycle(flames);
for (int z = 0; z < ctr - 1; z++) {
    res = iterator.next();
}
Community
  • 1
  • 1
Frankie
  • 24,627
  • 10
  • 79
  • 121
  • Even more elegant is to use infinite/cyclic iterators, like this: http://stackoverflow.com/questions/2622591/is-an-infinite-iterator-bad-design – lbalazscs Sep 11 '13 at 16:35
  • @lbalazscs thanks for comment. Did some testing and updated accordingly. – Frankie Sep 11 '13 at 17:06
5

You should use % to force the index stay within flames.length so that they make valid index

int len = flames.length;
for(int z = 0; z < ctr-1; z++){
      res = (flames[z % len]);
      jLabel1.setText(String.valueOf(res));
}
sanbhat
  • 17,522
  • 6
  • 48
  • 64
2

You can try the following:-

char res;
int ctr = 10
char[] flames = {'F','L','A','M','E','S'};
int n = flames.length();
for(int z = 0; z < ctr-1; z++){
    res = flames[z %n];
    jLabel1.setText(String.valueOf(res));
}
Rahul Tripathi
  • 168,305
  • 31
  • 280
  • 331
1

Here is how I would do this:

String flames = "FLAMES";
int ctr = 10;

textLoop(flames.toCharArray(), jLabel1, ctr);

The textLoop method:

void textLoop(Iterable<Character> text, JLabel jLabel, int count){
    int idx = 0;
    while(true)
        for(char ch: text){
            jLabel.setText(String.valueOf(ch));
            if(++idx < count) return;
        }
}

EDIT: found a bug in the code (idx needed to be initialized outside the loop). It's fixed now. I've also refactored it into a seperate function.

AJMansfield
  • 4,039
  • 3
  • 29
  • 50