Solved

static block with objects- good design?

Posted on 2011-03-20
22
341 Views
Last Modified: 2012-06-27
hi guys

Is this a good coding design ?


public class FIXMessageEncoder implements MessageEncoder {

    private static final Set<Class<?>> TYPES;
    private final String charsetEncoding;
   
    static {
        Set<Class<?>> types = new HashSet<Class<?>>();
        types.add(Message.class);
        types.add(String.class);
        TYPES = Collections.unmodifiableSet(types);
    }
 public FIXMessageEncoder() {
        charsetEncoding = CharsetSupport.getCharset();
    }
   
    public Set<Class<?>> getMessageTypes() {
        return TYPES;
    }
....

}



A static block with instantiation of objects.
 Will the GC be able to clean up the
objects within static block?
0
Comment
Question by:royjayd
  • 6
  • 5
  • 5
  • +3
22 Comments
 
LVL 92

Expert Comment

by:objects
ID: 35178299
thats fine
makes no differences (to the gc'or) where it is initialised
0
 
LVL 47

Assisted Solution

by:for_yan
for_yan earned 100 total points
ID: 35178328
0
 

Author Comment

by:royjayd
ID: 35178368
So are static fields a bad choice since they wont be garbage collected unless the class loader itself is garbage collected? (according to the article)

So i am guessing with in the static block having this instantiation is good :  
Set<Class<?>> types = new HashSet<Class<?>>();

because 'types' can be garbage collected, correct?

thx

0
 
LVL 92

Expert Comment

by:objects
ID: 35178384
Whether to use static or not is not based on garbage collection. It is based on whether each instance needs its own copy of the property

> Set<Class<?>> types = new HashSet<Class<?>>();

that would create a new set of type for every instance of the class.
Unless that is required then its not a good idea.
0
 
LVL 92

Expert Comment

by:objects
ID: 35178391
You should try and avoid storing huge objects as static vars, but in your case its small so is fine

> So i am guessing with in the static block having this instantiation is good :  
> Set<Class<?>> types = new HashSet<Class<?>>();

sorry misunderstood that above. Thats fine
thats just a local var, the fact that the code is in a static block makes no difference to how it is treated.
0
 
LVL 47

Expert Comment

by:for_yan
ID: 35178404
@object
Sorry, didn't understand:
you still mean that
types (as it is given in the code in original post)
 is static and will be created one
such set per class (not per instance)?

Nevertheless, it is not a problem from the point of view
of garbage collection.

Correct ?

0
 

Author Comment

by:royjayd
ID: 35178411
So are static fields a bad choice since they wont be garbage collected unless the class loader itself is garbage collected?  
0
 
LVL 92

Expert Comment

by:objects
ID: 35178423
> types (as it is given in the code in original post)
> is static and will be created one
> such set per class (not per instance)?

No, misunderstood. See my subsequent comment

> So are static fields a bad choice since they wont be garbage collected unless the class loader itself is garbage collected?  

Not really. They are more a bad choice because they are (pseudo) global variables. Depends how you are using them.

From a garbage collection point of view you should be careful with large static variables. Though static variables are typically required for the lifetime of the application so are not going to need garbage collection anyway.
0
 
LVL 47

Expert Comment

by:for_yan
ID: 35178424
On the other hand if you have many instances they will not be created
for each instance.
So probably decision should rather be made based
on whether the field needs
 to be specific per each instance or common for all instances
of the class.
0
 
LVL 47

Expert Comment

by:for_yan
ID: 35178436
So static block is different from static field,
so that in the above code "TYPES" will be unique
and "types" will belong to each instance - correct?
0
 
LVL 92

Expert Comment

by:objects
ID: 35178437
> so that in the above code "TYPES" will be unique

correct

> and "types" will belong to each instance - correct?

no, types is just a local var
0
Free Trending Threat Insights Every Day

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

 
LVL 47

Expert Comment

by:for_yan
ID: 35178446
Ah, yes, types will only be defined within the block,
but still will be once per class.
I guees, I now understand.
0
 
LVL 26

Expert Comment

by:dpearson
ID: 35178472
As has been pointed out the GC will only clean this up when the class itself goes out of scope.

But the general question of "is this good coding design".  The short answer is probably not.

The much simpler pattern for this code is just:

    public Set<Class<?>> getMessageTypes() {
        Set<Class<?>> types = new HashSet<Class<?>>();
        types.add(Message.class);
        types.add(String.class);
        return types;
    }

This has several advantages:
1) It's less code
2) It doesn't do any work during class loading - so if it throws an exception you'll get it when you try to access the object, not at a weird point during the loading of the class (which is very hard for the user to understand).
3) It doesn't rely on Collections.unmodifiableSet(types) for correctness.  The previous piece of code needs this to be present or it's wrong (since the returned object could otherwise be modified).  OK - so if the call is in there why is this still poor design?  Because if you ask 100 Java developers what "Collections.unmodifiableSet" does you'll find 50 don't know.  And that's not their fault - it's not a commonly used part of the language, so using it is bad practice.

I could go on...

Doug
0
 
LVL 26

Expert Comment

by:dpearson
ID: 35178485
Oops - I should have pointed out the downsides of my solution too:

1) It's (very slightly) slower
2) It creates a new object each type it's called.

Both of these are negligible in any real system.  If you're trying to optimize to this level you're doing something wrong.  Never be afraid to create and destroy objects in a language like Java - it's optimized for just that (unlike say C++ where this was very expensive).

Doug
0
 
LVL 86

Expert Comment

by:CEHJ
ID: 35179258
There's no real connection between garbage collection and static declarations. Such declarations should be kept to a minimum as they can lead to procedural design, which is against the grain of Java and good design practice
0
 
LVL 10

Assisted Solution

by:gordon_vt02
gordon_vt02 earned 50 total points
ID: 35180947
@dpearson
It is not bad practice at all to use Collections.unmodifiableSet() just because "it's not a commonly used part of the language."  I tend to use it quite often for exactly the type of situation in this question.  The standard libraries are there to be used and, while it is a good idea to know what's going on under the hood, it isn't necessary to understand the implementation details in order to use the class.  In fact, when you need an immutable set, using Collections.unmodifiableSet() IS best practice rather than reinventing the wheel and writing your own wrapper class.
0
 
LVL 26

Expert Comment

by:dpearson
ID: 35182692

gordon_vt02:
@dpearson
It is not bad practice at all to use Collections.unmodifiableSet() just because "it's not a commonly used part of the language."  I tend to use it quite often for exactly the type of situation in this question

Hey Gordon,

It's perfectly fine to use a feature like this when it's needed.  My point is that there's a perfectly good alternative solution here which doesn't require this sort of more advanced language usage and so you should prefer the simpler solution.

Doug

P.S. Remember the signature exposed by this class is:

public Set<Class<?>> getMessageTypes()

which gives no clue that the object is (a) shared and (b) semi-immutable (in the sense that it will throw an exception if you try to mutate it).
0
 

Author Comment

by:royjayd
ID: 35206829
I think we are deviating from the topic. the question was
>>static block with objects- good design?  

Here are my observations:
1.
Using a static block is better option than using static

public class abc{

static {
        Set<Class<?>> types = new HashSet<Class<?>>(); -- garbage collected
       
    }
}

public class abc{
public static Set<Class<?>> types = new HashSet<Class<?>>(); --not garbage collected
}


2. Static variables should not contain huge objects since they dont get garbage collected.

something like
public class abc{
public static list types = new ArrayList(1000000);  -- not recommended
..
}


3.so there is a relation between static variables and garbage collection as i understnad. If i have
something like
public class abc{
public static Object types1 = new Object(x);
public static Object types2 = new Object(x,y);
public static Object types3 = new Object(x,g,t);
public static Object types4 = new Object(y,t,r,e);

....
public static Object types100 = new Object(x,v,b,n);
}

all these 100 static varibles stay in the memory and the GC is not able to garbage collect them until the class is unloaded.
So the less the number of static variables the better it is for performance.


Are 1,2,3 correct?


Thanks.

0
 
LVL 26

Accepted Solution

by:
dpearson earned 250 total points
ID: 35208107

1.
Using a static block is better option than using static

public class abc{

static {
        Set<Class<?>> types = new HashSet<Class<?>>(); -- garbage collected
       
    }
}

public class abc{
public static Set<Class<?>> types = new HashSet<Class<?>>(); --not garbage collected
}

Yes this is correct - the first "types" object shown here can be garbage collected.  Although in the code you originally posted, the static code block was used to initialize a static variable.  So the "types" in your original post will *not* be garbage collected.

Indeed - usually a static code block is used to initialize static variables (it can't access any other types of variables) and the static variables will not be garbage collected.


2. Static variables should not contain huge objects since they dont get garbage collected.

something like
public class abc{
public static list types = new ArrayList(1000000);  -- not recommended
..
}

That's probably a good rule of thumb - but there are no absolutes here.  In some cases this can be reasonable, but it's good to avoid in general.



3.so there is a relation between static variables and garbage collection as i understnad. If i have
something like
public class abc{
public static Object types1 = new Object(x);
public static Object types2 = new Object(x,y);
public static Object types3 = new Object(x,g,t);
public static Object types4 = new Object(y,t,r,e);

....
public static Object types100 = new Object(x,v,b,n);
}

all these 100 static varibles stay in the memory and the GC is not able to garbage collect them until the class is unloaded.
So the less the number of static variables the better it is for performance.

Yes they won't be garbage collected if they're statically allocated.

In general staying away from static variables is a good idea.  They should usually only be used when there's no other reasonable alternative design.

Doug
0
 

Author Comment

by:royjayd
ID: 35208323
>>>So the "types" in your original post will *not* be garbage collected.
I think you mean "TYPES" in my original post will *not* be garbage collected

thx.
0
 
LVL 26

Expert Comment

by:dpearson
ID: 35209009

>>>So the "types" in your original post will *not* be garbage collected.
I think you mean "TYPES" in my original post will *not* be garbage collected

Well yes, TYPES won't be - but since that points to "types", that also won't be garbage collected:

        Set<Class<?>> types = new HashSet<Class<?>>();
        types.add(Message.class);
        types.add(String.class);
        TYPES = Collections.unmodifiableSet(types);  <-- TYPES holds reference to types here, so neither is gc'd

Doug
0
 
LVL 92

Assisted Solution

by:objects
objects earned 100 total points
ID: 35210968
just because something can't be gc'ed doesn't necessarily mean its bad.
It may not need collecting, for example if its needed for the lifetime of your application.
gc is not really the primary factor when deciding whether something should be static or not
0

Featured Post

Why You Should Analyze Threat Actor TTPs

After years of analyzing threat actor behavior, it’s become clear that at any given time there are specific tactics, techniques, and procedures (TTPs) that are particularly prevalent. By analyzing and understanding these TTPs, you can dramatically enhance your security program.

Join & Write a Comment

Suggested Solutions

Title # Comments Views Activity
scoreUp challenge 14 48
Receive file in Servlet 1 36
count11 challenge 6 48
JDeveloper 12c for 32 bit 4 36
Java contains several comparison operators (e.g., <, <=, >, >=, ==, !=) that allow you to compare primitive values. However, these operators cannot be used to compare the contents of objects. Interface Comparable is used to allow objects of a cl…
Introduction This article is the first of three articles that explain why and how the Experts Exchange QA Team does test automation for our web site. This article explains our test automation goals. Then rationale is given for the tools we use to a…
Viewers learn about the scanner class in this video and are introduced to receiving user input for their programs. Additionally, objects, conditional statements, and loops are used to help reinforce the concepts. Introduce Scanner class: Importing…
Viewers will learn about the different types of variables in Java and how to declare them. Decide the type of variable desired: Put the keyword corresponding to the type of variable in front of the variable name: Use the equal sign to assign a v…

746 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

13 Experts available now in Live!

Get 1:1 Help Now