design issue generics/factory/abstract class

Question

I'm pretty new to Java programming and I've got a design question. At the moment what I got is the following:

public class MyFactory {

    private MyFactory(){
        //hidden constructor
    }

    public static ImageFilter getInstance(String filterType){
        if(filterType == “foo“){
            return new FooFilter();
        }
        return null;
    }
}

public abstract class ImageFilter {
    public abstract Bitmap filterImage(byte[] data);

    //some other stuff
}

public class FooFilter extends ImageFilter {
    public C filterImage(byte[] data){
        //want to apply filterImageA or filterImageB depending what I put in
        //at (*) and (**)
    }

    private A filterImageA(byte[] data){
        //
    }

    private B filterImageB(byte[] data){
       //
    }
}

void main(byte[] data) {
    ImageFilter bar = MyFactory.getInstance(“foo“);
    BitmapType1 myBitmap = bar.filterImage(byte[] data);   //(*)
    BitmapType2 myBitmap2 = bar.filterImage(byte[] data);   //(**)
}

In the main method I know what the resulting type is. If it is BitmapType1 I have to apply filterImageA. If it is BitmapType2 then I have to use filterImageB. Is there a generic way to do that? I read about generics, but have no idea on how to use them in this particular case. I hope thats not too confusing. Perhaps the whole approach is crap. Feel free to suggest a better one!

fps · Accepted Answer · 2015-01-18T18:15:42.030

1

@Dima's answer is right. But you could also make ImageFilter more generics-friendly:

public class MyFactory {

    private MyFactory(){
        //hidden constructor
    }

    public static ImageFilter getInstance(String filterType){
        if(filterType == “foo“){
            return new FooFilter();
        }
        return null;
    }
}

public abstract class ImageFilter {
    public abstract <T extends Bitmap> T filterImage(byte[] data, Class<T> clazz);

    //some other stuff
}

public class FooFilter extends ImageFilter {
    public <T extends Bitmap> T filterImage(byte[] data, Class<T> clazz){
        if (BitmapType1.class.isAssignableFrom(clazz)) {
            return this.filterImageA(data);
        } else if (BitmapType2.class.isAssignableFrom(clazz)) {
            return this.filterImageB(data);
        }
        return null; // or better throw runtime exception
    }

    private BitmapType1 filterImageA(byte[] data){
        //
    }

    private BitmapType2 filterImageB(byte[] data){
       //
    }
}

void main(byte[] data) {
    ImageFilter bar = MyFactory.getInstance(“foo“);
    BitmapType1 myBitmap = bar.filterImage(byte[] data, BitmapType1.class);
    BitmapType2 myBitmap2 = bar.filterImage(byte[] data, BitmapType2.class);
}

Note: if either BitmapType1 inherits (directly or not) from BitmapType2 or viceversa, you would need to check the most concrete class of the hierarchy first:

        if (BitmapType1.class.isAssignableFrom(clazz)) { // BitmapType1 type more concrete
            return this.filterImageA(data);
        } else if (BitmapType2.class.isAssignableFrom(clazz)) { // BitmapType2 type more general
            return this.filterImageB(data);
        }

edited Jan 18 '15 at 18:15

answered Jan 15 '15 at 18:23

fps

33,623
8
55
110

I tried it your way but i end up with an error message like: `"incompatible conditional operand types Class and Bitmap"`. The only common super class both image types share is Object. Could that be the problem? Is it possible to get around this? – Tim Schulz Jan 16 '15 at 06:57
Is it a compilation error? In which line does the error occur? If you are using Eclipse, please see [this question](http://stackoverflow.com/questions/2551337/instanceof-incompatible-conditional-operand-types), especially the 2nd answer. It might be helpful. – fps Jan 16 '15 at 12:04
Yes it is a compile time error. I'm using Eclipse, but Bitmap is imported. Just for fun I tried something like `T test = null` and `if(clazz instanceof test)...` and the Error disappeared. Unfortunately the condition never went true. I don't know if I really understand the concept of `Class clazz` in this case. Is it actually an instance of T? – Tim Schulz Jan 16 '15 at 17:26
@Tim An instance of Class in Java reflects the runtime type of an instance of class T (i.e. it has name, package name, fields, etc). Do BitmapType1 and BitmapType2 classes inherit from Bitmap class? Are you importing BitmapType1 and BitmapType2 as `import some.package.BitmapType1;` and `import some.package.BitmapType2;` as well? – fps Jan 16 '15 at 18:39
hrm then Java should know it at compile time as well. Thats strange. Actually the both image types are: `Bitmap` and `YuvImage` (Android). They do not share a common super class except `Object`. – Tim Schulz Jan 16 '15 at 19:39
@Tim Please change `if (clazz instanceof YuvImage)` to `if (YuvImage.class.isAssignableFrom(clazz))`. Same with the other if, but using `Bitmap`. – fps Jan 16 '15 at 19:55

score 0 · Answer 2 · answered Jan 15 '15 at 17:54

0

Instead of filterImageA and filterImageB, create two different filters: one having it's filterImage do what filterImageA does, and the other for filterImageB. Then in your main, where you know whether you want "A" or "B", get the proper filter by name from the factory, and invoke filterImage on it.

answered Jan 15 '15 at 17:54

Dima

39,570
6
44
70

That's a good idea. It's just filterImageA and filterImageB do almost the same and I'd like to have them grouped together. Essentially it's the same filter applied on two different file formats using different buffers and so on. Sorry for not making that clear enough! – Tim Schulz Jan 15 '15 at 18:05
You can make them have a common superclass, that would contain the common logic, and override only the parts that differ in concrete subclasses. – Dima Jan 15 '15 at 18:11

score 0 · Answer 3 · answered Jan 15 '15 at 18:06

I have multiple remarks on this snippet of code:

The MyFactory class could have a registration method like add(String bitmapType, String filterType, ImageFilter filter) so that this can be kept dynamic. Create a bean that holds the two values as a key for a Map (be careful about equals() and hashCode() to hold them and you are done. Effectively, you will have an ImageFilter for each single bitmapType and filter processing.
This is a personal choice, but I would write ImageFilter as an interface and - if needed - add an AbstractImageFilter using a template method to encapsulate pre/post common behaviour.

design issue generics/factory/abstract class

3 Answers3