Question

I'm writing a function where I'm essentially doing the same thing over and over. I have the function listed below

public String buildGarmentsString(List<Garment> garments)
{
    StringBuilder garmentString = new StringBuilder(10000);
    for(int i=0;i<4;i++)
    {
        garmentString.append(this.garmentProductId(i,garments.get(i).getProductId()));
        garmentString.append(this.garmentColor(i,garments.get(i).getColor()));
        for(int j=0;j<garments.get(i).getSizes().size();j++)
        {
            //check xxsml
            if(garments.get(i).getSizes().get(j).getXxsml() >0)
            {
                garmentString.append(this.garmentSizes(i, Size.xxsml(),garments.get(i).getSizes().get(j).getXxsml()));

            }

            //check xsml
            if(garments.get(i).getSizes().get(j).getXsml() > 0)
            {
                garmentString.append(this.garmentSizes(i,Size.xsml(),garments.get(i).getSizes().get(j).getXsml()));
            }

            //check sml
            if(garments.get(i).getSizes().get(j).getSml() > 0)
            {
                garmentString.append(this.garmentSizes(i,Size.sml(),garments.get(i).getSizes().get(j).getSml()));
            }

            //check med
            if(garments.get(i).getSizes().get(j).getMed() > 0)
            {
                garmentString.append(this.garmentSizes(i,Size.med(),garments.get(i).getSizes().get(j).getMed()));
            }

            //check lrg
            if(garments.get(i).getSizes().get(j).getLrg() > 0)
            {
                garmentString.append(this.garmentSizes(i,Size.lrg(),garments.get(i).getSizes().get(j).getLrg()));
            }

            //check xlrg
            if(garments.get(i).getSizes().get(j).getXlg() > 0)
            {
                garmentString.append(this.garmentSizes(i,Size.xlg(),garments.get(i).getSizes().get(j).getXlg()));
            }

            //check xxlrg
            if(garments.get(i).getSizes().get(j).getXxl() >0)
            {
                garmentString.append(this.garmentSizes(i,Size.xxlg(),garments.get(i).getSizes().get(j).getXxl()));
            }

            //check xxxlrg
            if(garments.get(i).getSizes().get(j).getXxxl() >0)
            {
                garmentString.append(this.garmentSizes(i,Size.xxxlg(),garments.get(i).getSizes().get(j).getXxxl()));
            }


        }
    }
}

This is my garmentSizes function:

public String garmentSizes(int garmentNumber, String size,int numberToSend)
{
    String garmentSizes = "&garment["+garmentNumber+"][sizes]["+size+"]="+numberToSend;
    return garmentSizes;
}

I'm trying to figure out how I can get this done with a lot less code. I've read that with functional programming you can do things like pass in functions to parameters to other functions. After doing some reading online, I think I want to do something like this but I'm not sure how or what the best approach would be.

I have done some reading here on stack overflow and I've seen people mention using either the Command pattern or FunctionalJava or LambdaJ for trying to approximate this feature in Java. I've read over the documentation for the two libraries and read the Wikipedia Article on the Command Pattern, but I'm still not sure how I would use any of those to solve my particular problem. Can somebody explain this to me? As somebody that has never done any functional programming this is a bit confusing.

Was it helpful?

Solution

  • You could use local variables to decrease the amount of repetition. Say bySize = garments.get(i).getSizes().get(j) for example.
  • instead of size.getXxsml(), size.getXsml() etc. you could use an enum for sizes and loop on sizes.

The whole thing would then look like:

for(int j=0;j<garments.get(i).getSizes().size();j++) {
    bySize = garments.get(i).getSizes().get(j);
    for (Size s : Size.values()) {
        if (bySize.get(s) > 0)  {
            garmentString.append(garmentSizes(i, s, bySize.get(s)));
        }
    }
}

The bySize.get(s) method could be implemented either with a switch that directs to the right method or directly in the enum and you could get rid of the getXsml etc. methods.

OTHER TIPS

The only thing which differs between all your checks is this:

getXxsml/xxsml, getXsml/xsml, getSml/sml, etc.

If you could pass these values (as strings) to some upper-level method, and if
that upper-level method could eval i.e. execute these strings, then you can just
have an array of these values and pass that array to that upper-level method.

In Java, you can do something similar with reflection.
All these checks could indeed be simplified to much less
code through the use of reflection.

Look at:
java.lang.Class
java.lang.reflect.Method
java.lang.reflect.Field
java.lang.reflect.Constructor
and you will see what I mean.

From your code it appears that some Class has the following methods:

 xxsml(), xsml(), sml(), med(), ..., xxxlg()

to get the amounts (?) available for each size.

You can design your data better, like this:

  • Have a "Size" type, that enumerates all sizes (could be Enum or some class with attribute String key)
  • Have a method that returns a List of all known sizes.
  • replace the above methods with amountFor(Size) This could be backed by a Map<Size, Integer>

For backward compatibility, you could rewrite the old methods along the lines:

int xxsml() {
    return amountFor(Size.XXSML);  // assuming you have a singleton instance
                                   // for each well known size
}

Of course, in getGarmentString, you would then loop through the List of all known sizes:

for (Size sz : Size.getAllKnownSizes()) {
    if (garments.get(i).getSizes().get(j).amountFor(sz) > 0) {
          ... do whatever must be done here
    }
 }
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top