2

I have a whole bunch of classes that define these two constants, eg:

public class Face
{
    public static final int LUMP_INDEX = 1;
    public static final int SIZE = 20;

    blah blah
}

public class Edge
{
    public static final int LUMP_INDEX = 5;
    public static final int SIZE = 32;

    blah blah
} 
etc.

At the moment, i have a function for each one to create an array of that class, using the 2 constants defined in the class.

private Face[] createFaces(RandomAccessFile in)
{
    int numFaces = doSomeCalculations(Face.LUMP_INDEX, Face.SIZE);
    Face[] faces = new Face[numPlanes];

    blahblah;
    for(int i = 0; i < numFaces; i++)
        faces[i] = new Face();

    return faces;
}

A bit silly to have a create function for every class. The only thing that changes is the class type. So i wanted to create a genertic method that would work with any of the classes above. Something like:

private T[] create(RandomAccessFile in, Class T)
{
    int num = doSomeCalculations(T.LUMP_INDEX, T.SIZE);
    T[] faces = new T[numPlanes];

    blahblah;
    for(int i = 0; i < num; i++)
        faces[i] = new T();

    return faces;
}

However I'm not sure how to do it properly. Any help would be appreciated. Thanks.

Improve this question
3
  • 1
    As Borgwardt says, although instead of reading fields with reflection you could define an interface with two get methods. Commented Mar 26, 2011 at 12:03
  • @Johan: but then you'd need an instance of the class to call the methods, since they can't be static and part of the Class object. Commented Mar 26, 2011 at 12:05
  • @Borgwardt, exactly, I meant as a possible step to refactor the code. Perhaps by creating a new T first, then using t.setNumPlaces(n). Or store configuration elsewhere, e.g., UtilClass.getLumpIndex(Faces.class). Commented Mar 26, 2011 at 12:08

4 Answers 4

Reset to default
7

The only way that could be made to work is by using reflection to read the constants through the class object and Array.newInstance() to create the array. Oh, and the method signature would have to look like this:

private <T> T[] createFaces(RandomAccessFile in, Class<T> clazz)

Edit An alternative solution would be to keep the constants in a map keyed by class rather than as static fields. Java just isn't dynamic enough, especially on the class level, to do it your way cleanly.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

Ok thanks, i might have to go the route of storing the constants elsewhere, or passing them in as params. How would i instantiate the clazz that's passed in, since i can't do a[i] = new T();
@terryhau: you can use clazz.newInstance(), but only if all the classes have parameterless constructors.
Encouraging reflection in situations like this is utterly irresponsible. Shame on you.
@Tom: encouraging the use of design patterns as a silver bullet in situations where they don't improve anything is no better.
1

There's no excuse for using reflection in a situation like this. Bung in an Abstract Factory or similar.

Improve this answer

5 Comments

I'm not sure what an Abstract Factory is, but i'll have a look.
@terryhau See the GoF book (Gang of Four, "Design Patterns").
Thanks. If I'm understanding the Abstract Factory design pattern correctly, i would have a Factory interface, and I'd have a concrete factory for each of my class types I'm creating. So each concrete factory is going to have a create function that is copy pasted, and i end up with the same problem right? Or am i not getting the design pattern.
@terryhau: you're right, an abstract factory would be theoretically cleaner but would probably lead to even worse code duplication in practice.
@terryhau Once you have it in abstract factory form or similar, it is trivial to factor out the common code.
1

Potentially you could do this by having both (all?) of the items you want to create extend some superclass "Array-Able Object"

But provided your "blablabla" in createFaces is not the same, you gain nothing and back yourself into a corner with that extension.

I'd look into whether or not you really need these different classes which all do the same thing, or whether there's some more general fix you could look into. If the only difference between the two is their static variables, for instance- then you should move those to a single location, say- a resource file.

If all you have the same, though, are those "create array" sections of this code, then separating or somehow linking them seems unnecessary at best.

Improve this answer

1 Comment

Yes, all the create functions are exactly the same, except for the class type.
0

You can always use the simpler form:

<T> T[] create(RandomAccessFile in, Class<T> clazz, int lumpIndex, int size)
{
    int numPlanes = doSomeCalculations(lumpIndex, size);
    T[] faces = new T[numPlanes];

    blahblah;

    return faces;
}

And use it like

Face[] faces = create(in, Faces.class, Face.LUMP_INDEX, Face.SIZE

It is neither as professional as using an abstract factory, nor as silly (and dangerous!) as having the same function over and over again. You can still make the mistake of calling it using

Face[] faces = create(in, Faces.class, Edge.LUMP_INDEX, Edge.SIZE)

But this is a mistake that it is easy to point.

Improve this answer

2 Comments

I tried to do it this way, but i'm not sure how to instantiate the clazz thats passed in.
To instantiate one element: clazz.newInstance(). Create an array is more complex, you need something like T[] a = (T[]) Array.newInstance(clazz, size); Since the cast to (T[]) is unsafe, you will need to use an annotation like @SuppressWarnings("unchecked") over your method

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.