tags:

views:

166

answers:

4
static private       ArrayList   seriesColors      = new ArrayList();

public Audiogram(int widthParm, int heightParm)
            throws Exception
    {
        super(widthParm, heightParm);
        seriesColors.add(new Color(  0,   0, 255));

        // Set the default settings to an industrial audiogram
        setType(INDUSTRIAL_AUDIOGRAM);
    }

I have some five methods like this, but would like to know whether this code above could cause memory leakage since the seriesColors is static.

If there is a memory leakage, then what is the solution for that?

private static final ColorModel  rgbModel  = ColorModel.getRGBdefault();

public void setPixels(int x, int y, int w, int h, ColorModel model, int[] pixels, int off, int scansize )
    {
        if ( model == rgbModel ) {
            try {
                encodePixelsWrapper( x, y, w, h, pixels, off, scansize );
                }
            catch ( IOException e ) {
                iox = e;
                stop();
                return;
                }
            }
            else {
                int[] rgbPixels = new int[w];
                for ( int row = 0; row < h; ++row ) {
                    int rowOff = off + row * scansize;
                    for ( int col = 0; col < w; ++col ) {
                        rgbPixels[col] = model.getRGB( pixels[rowOff + col] );
                        }
                    try {
                        encodePixelsWrapper( x, y + row, w, 1, rgbPixels, 0, w );
                        }
                    catch ( IOException e ) {
                        iox = e;
                        stop();
                        return;
                        }
                    }
                }
    }

 public static ColorModel getRGBdefault() {
    if (RGBdefault == null) {
        RGBdefault = new DirectColorModel(32,
                          0x00ff0000,   // Red
                          0x0000ff00,   // Green
                          0x000000ff,   // Blue
                          0xff000000    // Alpha
                          );
    }
    return RGBdefault;
    }

Out of these two code which one has serious flaws?

+1  A: 

why would you make two post's for the same question? As for your question in general static member variables can cause memory leaks if not handled properly. With properly i mean that those variables live as long as the app lives and you have to take care that for instance an arraylist deletes items which are no longer needed.

kukudas
A: 

A collection utilising weak references is generally the solution to the kind of problem in your first example.

See: http://download.oracle.com/javase/1.4.2/docs/api/java/lang/ref/WeakReference.html

John Pickup
Probably not; but please explain what I have missed...
KarlP
Indeed, I should read the question properly. Weak references are completely useless in this scenario as the Color will be dropped from the collection as soon as it goes out of scope. They are of course a good solution where you want a collection for accessing some cached objects but don't want to prolong their lifetime, i.e. they only live in the cache for as long as some other object is holding a reference.
John Pickup
+1  A: 

Static variables are shared between all the instances of a class. (An instance is created using the "new" operator.)

In these examples; it's probably not a good idea to use a static (instance variable) to store colours in, as the instances will interfere with each other. That variable should be changed to an "ordinary" instance variable.

The final static colorModel in the second example is perfectly fine; It's an immutable object (at least the interface is immutable) and the methods are most likly threadsafe and can be used by a lot of instances at the same time.

Note that the first problem is not a "memory leak". You might say that hte instances unintentionally is leaking data between them, but's its not a classic memory leak where active objects are unintenonally keeping references to "dead" objects.

IF seriesColors for some reason should contain a color for each instance created. (Which is probably a stupid design) the access to the arraylist mustbe syncronized in some way. But I think that is out of scope here...

KarlP
A: 

With regards to your first question, it's hard to say whether seriesColors would create a substantial memory issue without knowing more about how the rest of the program works. E.g., Are objects ever removed from seriesColors? How often is a new Audiogram created? How many Audiograms will be created over the runtime life of the program? Etc.

Bill Horvath II