Reputation: 1

avoid nesting foreach loop in java

It's been about 20 years since I have taken a coding class. I pick it up again from time to time for fun, but my code is clunky & inefficient.

I have an array with just over 400 elements. I currently have nested foreach loops operating over that array.

import acm.program.ConsoleProgram;
import java.awt.Color;
import acm.io.IODialog;
import java.text.*;
import static java.lang.Math.*;
import java.util.*;

/** Tests to see if user color matches sample colors */

public class test extends ConsoleProgram

{

   //defining sample colors

   Color[] dmc =
   {
      new Color(255,255,255),
      new Color(148,91,128),
      new Color(206,148,186),
      new Color(236,207,225),
      new Color(243,218,228),

   };

 public void run()
 {
    average();

 }

 //averages three colors, then tests for match to given color

 public void average()
 {

    //asks for user color
    IODialog dialog = new IODialog();
    int stitchRed= dialog.readInt("Enter red value: ");
    int stitchGreen= dialog.readInt("Enter green value: ");
    int stitchBlue= dialog.readInt("Enter blue value: ");
    Color stitchColor= new Color(stitchRed,stitchGreen,stitchBlue);

    //gets averages for dmc colors
    for (Color i:dmc)
    {
       for (Color j:dmc)
       {
          for (Color k:dmc)
          {
              int indexI = Arrays.asList(dmc).indexOf(i);
              int indexJ = Arrays.asList(dmc).indexOf(j);
              int indexK = Arrays.asList(dmc).indexOf(k);
              if(indexI <= indexJ && indexJ <= indexK)
              {
                 int iRed = i.getRed();
                 int jRed = j.getRed();
                 int kRed = k.getRed();
                 int iGreen = i.getGreen();
                 int jGreen = j.getGreen();
                 int kGreen = k.getGreen();
                 int iBlue = i.getBlue();
                 int jBlue = j.getBlue();
                 int kBlue = k.getBlue();
                 int redAverage = (iRed+jRed+kRed)/3;
                 int greenAverage = (iGreen+jGreen+kGreen)/3;
                 int blueAverage = (iBlue+jBlue+kBlue)/3;
                 Color colorAverage = new Color(redAverage,greenAverage,blueAverage);

                 //tests to see if any thread average equals user color
                 if (colorAverage.equals(stitchColor))
                 {
                    println("The color match is: " + i + ", " + j + ", " + k);
                 }
            }
         }
       }
     }

     println("no more matches");
   }
 }

This compiles fine, but runs REALLY slowly.

Is there a more efficient way to do this?
Possibly a way to get around nesting - something to the effect of:

for( Color i,j,k:dmc)

Upvotes: 0

Answers (5)

c0der

Reputation: 18812

(I can't test and debug the code since it is not an SSCCE, so I appologies for any errors)
Consider the following, see comments for details:

Color[ ] dmc = {element1, element2, ...};

for( int i =0; i < dmc.length ; i++ )
 {
    
    //iterate only on values >= i 
   for( int j = i; j< dmc.length; j++ )
   {
       //loop only on values >= j
       for( int k = j; k < dmc.length ; k++ )
       {
        
           //not needed, you already have i, j, k 
           //int indexI = Arrays.asList(dmc).indexOf(i);
           //int indexJ = Arrays.asList(dmc).indexOf(j);
           //int indexK = Arrays.asList(dmc).indexOf(k);
          
           
           //condition is not needed. Allways true. 
           //if( indexI <= indexJ && indexJ <= indexK ) { }
           
           //bunch of code...

EDIT: A short demo to print out only the wanted incidences:

        public static void main(String[] args) {

            Color[ ] dmc = {Color.AQUA, Color.AZURE, Color.BEIGE, Color.BLACK, Color.BLUE,
                    Color.BROWN, Color.CHOCOLATE, Color.CYAN,  Color.CRIMSON, Color.FUCHSIA};

            for( int i =0; i < dmc.length ; i++ )
             {

                System.out.printf("\n >>>>>> i= %s", i);
                //iterate only on values >= i
               for( int j = i; j< dmc.length; j++ )
               {
                   System.out.printf("\n         j= %s",j);

                   //loop only on values >= j
                   for( int k = j; k < dmc.length ; k++ )
                   {

                     System.out.printf("\n            k= %s",k);
                   }
               }
             }

        }

EDIT II: Following the MCVE posted I modified it by adding fastEverage() method, which should do the same calculation as everage() with less iterations.
On the single set of colors I checked, both methods have the same output:

import java.awt.Color;
import java.util.Arrays;

/** Tests to see if user color matches sample colors */
public class Test

{

    //defining sample colors

    Color[] dmc =
        {
                new Color(255,255,255),
                new Color(148,91,128),
                new Color(206,148,186),
                new Color(236,207,225),
                new Color(243,218,228),

        };

    //averages three colors, then tests for match to given color
    public void average()
    {

        //asks for user color
        int stitchRed =  203; 
        int stitchGreen= 164;
        int stitchBlue= 189;

        Color stitchColor= new Color(stitchRed,stitchGreen,stitchBlue);

        //gets averages for dmc colors
        for (Color i:dmc)
        {
            for (Color j:dmc)
            {
                for (Color k:dmc)
                {
                    int indexI = Arrays.asList(dmc).indexOf(i);
                    int indexJ = Arrays.asList(dmc).indexOf(j);
                    int indexK = Arrays.asList(dmc).indexOf(k);
                    if((indexI <= indexJ) && (indexJ <= indexK))
                    {
                        int iRed = i.getRed();
                        int jRed = j.getRed();
                        int kRed = k.getRed();
                        int iGreen = i.getGreen();
                        int kGreen = k.getGreen();
                        int jGreen = j.getGreen();
                        int iBlue = i.getBlue();
                        int jBlue = j.getBlue();
                        int kBlue = k.getBlue();
                        int redAverage = (iRed+jRed+kRed)/3;
                        int greenAverage = (iGreen+ jGreen + kGreen)/3;
                        int blueAverage = (iBlue+jBlue+kBlue)/3;

                        Color colorAverage = new Color(redAverage,greenAverage,blueAverage);

                        //tests to see if any thread average equals user color
                        if (colorAverage.equals(stitchColor))
                        {
                            System.out.println("The color match is: " + i + ", " + j + ", " + k);
                        }

                    }
                }
            }
        }

        System.out.println("no more matches");
    }

    //averages three colors, then tests for match to given color
    public void fastEverage()
    {

        //asks for user color
        int stitchRed =  203; 
        int stitchGreen= 164;
        int stitchBlue= 189;
    
        Color stitchColor= new Color(stitchRed,stitchGreen,stitchBlue);

        Color colorI, colorJ, colorK;

        for( int i =0; i < dmc.length ; i++ )
        {

            colorI = dmc[i];

            //iterate only on values >= i
            for( int j = i; j< dmc.length; j++ )
            {

                colorJ = dmc[j];

                //loop only on values >= j
                for( int k = j; k < dmc.length ; k++ )
                {

                    colorK = dmc[k];

                    int iRed = colorI.getRed();
                    int jRed = colorJ.getRed();
                    int kRed = colorK.getRed();
                    int iGreen = colorI.getGreen();
                    int kGreen = colorK.getGreen();
                    int jGreen = colorJ.getGreen();
                    int iBlue = colorI.getBlue();
                    int jBlue = colorJ.getBlue();
                    int kBlue = colorK.getBlue();
                    int redAverage = (iRed+jRed+kRed)/3;
                    int greenAverage = (iGreen+ jGreen + kGreen)/3;
                    int blueAverage = (iBlue+jBlue+kBlue)/3;

                    Color colorAverage = new Color(redAverage,greenAverage,blueAverage);

                    //tests to see if any thread average equals user color
                    if (colorAverage.equals(stitchColor))
                    {
                        System.out.println("The color match is: " + colorI + ", " + colorJ + ", " + colorK);
                    }

                }
            }
        }

        System.out.println("no more matches");
    }

    public static void main(String[] args)
    {
        new Test().average();
        new Test().fastEverage();

    }
}

The run time difference between the 2 methods, on this test case, is significant:

run time of everage() in nano seconds 84752814

run time of fastEverage() in nano seconds 76497255

Upvotes: 1

David Pérez Cabrera

Reputation: 5068

It's equals to @C0der's answer, Just uses the same names for question's var.

    for (int indexI = 0; indexI < dmc.length; indexI++) {
        Color i = dmc[indexI];
        for (int indexJ = indexI; indexJ< dmc.length; indexJ++) {
            Color j = dmc[indexJ];
            for (int indexK = indexJ; indexK< dmc.length; indexK++) {
                Color k = dmc[indexK];
                //....
            }
        }
    }

EDITED:

public void average() {
    //asks for user color
    IODialog dialog = new IODialog();
    int stitchRed = dialog.readInt("Enter red value: ");
    int stitchGreen = dialog.readInt("Enter green value: ");
    int stitchBlue = dialog.readInt("Enter blue value: ");
    Color stitchColor = new Color(stitchRed, stitchGreen, stitchBlue);
    for (int indexI = 0; indexI < dmc.length; indexI++) {
        Color i = dmc[indexI];
        for (int indexJ = indexI; indexJ < dmc.length; indexJ++) {
            Color j = dmc[indexJ];
            for (int indexK = indexJ; indexK < dmc.length; indexK++) {
                Color k = dmc[indexK];
                int iRed = i.getRed();
                int jRed = j.getRed();
                int kRed = k.getRed();
                int iGreen = i.getGreen();
                int jGreen = j.getGreen();
                int kGreen = k.getGreen();
                int iBlue = i.getBlue();
                int jBlue = j.getBlue();
                int kBlue = k.getBlue();
                int redAverage = (iRed + jRed + kRed) / 3;
                int greenAverage = (iGreen + jGreen + kGreen) / 3;
                int blueAverage = (iBlue + jBlue + kBlue) / 3;
                Color colorAverage = new Color(redAverage, greenAverage, blueAverage);
                //tests to see if any thread average equals user color
                if (colorAverage.equals(stitchColor)) {
                    System.out.println("The color match is: " + i + ", " + j + ", " + k);
                }
            }
        }
    }

    System.out.println("no more matches");
}

Upvotes: 1

Tahseen Adit

Reputation: 184

Just an idea. You can split the search. There is a possibility to find the match within the first search. If so, then you don't need to search the rest for a match.If not, you can move forward to search for the match.

int i,j,k;    
for(i=0;i<100;i++){
   for(j=i;j<100;j++){
      for(k=j;k<100;k++){
         //..do stuffs.....//
       } 
   }
}
if(matchnotfound){
  for(i=100;i<200;i++){
   for(j=i;j<200;j++){
      for(k=j;k<200;k++){
         //..do stuffs.....//
      } 
    }
  }
}
 if(matchnotfound){
  for(i=200;i<300;i++){
   for(j=i;j<300;j++){
      for(k=j;k<300;k++){
         //..do stuffs.....//
      } 
    }
  }
}
 if(matchnotfound){
  for(i=300;i<400;i++){
   for(j=i;j<400;j++){
      for(k=j;k<400;k++){
         //..do stuffs.....//
      } 
    }
  }
}

Upvotes: 0

Tahseen Adit

Reputation: 184

You can try something like

for( x=1;x<401;x++ )
{
    Color j= dmc[x];
    Color k= dmc[x+1];
    int indexI = Arrays.asList(dmc).indexOf(i);
    int indexJ = Arrays.asList(dmc).indexOf(j);
    int indexK = Arrays.asList(dmc).indexOf(k);
    if( indexI <= indexJ && indexJ <= indexK )
    {
        //bunch of code...

or, you can try equals method defined in java.lang.Object. Hope this will help.

Upvotes: 0

Olav Trauschke

Reputation: 136

Considering you filter out many combinations in the if-statement, one optimization would be to use for-loops only iterating over the elements you actually want to process instead of for-each-loops. E.g. i <= j means you can start the iteration in j at i.

Further improvements may very well be possible depending on your intentions as stated in the comments.

Upvotes: 0

avoid nesting foreach loop in java

Answers (5)

Related Questions