Reputation: 1526
I have a badly created container object that holds together values of different java types(String, Boolean etc ..)
public class BadlyCreatedClass {
public Object get(String property) {
...;
}
};
And we extract values from it in this way
String myStr = (String) badlyCreatedObj.get("abc");
Date myDate = (Date) badlyCreatedObj.get("def");
I am forced to write some new code using this object and I am trying to see if there is clean way to do this. More specifically which method from the below is preferred ?
Explicit Cast
String myStr = (String) badlyCreatedObj.get("abc")
Date myDate = (Date) badlyCreatedObj.get("def");
Using generic cast
public <X> X genericGet(String property) {
}
public String getString(String property) {
return genericGet(property);
}
public Date getDate(String property) {
return genericGet(property);
}
Using Class.cast
<T> T get(String property, Class<T> cls) {
;
}
I have gone through several related questions on SO Java generic function: how to return Generic type , Java generic return type all of them seem to say the such typecasting is dangerous, ALthough I dont see much difference between the three, given this which method would you prefer ?
Thanks
Upvotes: 19
Views: 1240
Reputation: 70564
The generic cast approach causes the compiler to emit an unchecked warning. An unchecked warning indicates that the cast is question is not (fully) checked at runtime, i.e. it may succeed, even if the value is not of the proper type. This can cause a variable to hold a value not compatible with its declared type, a situation the Java Language Specification calls heap pollution.
The following program demonstrates this:
class Holder {
Object value;
Holder(Object value) {
this.value = value;
}
<T> T get() {
return (T) value;
}
}
class C<T> {
T value;
C(Holder h) {
value = h.get();
}
}
public class Test {
public static void main(String [] args) throws IOException {
Holder holder = new Holder("Hello");
C<Integer> c = new C<Integer>(holder);
System.out.println("I just put a String into a variable of type Integer");
// much later, possibly in a different part of your program
c.value.longValue(); // throws ClassCastException
}
}
Therefore, I strongly recommend using a checked cast. Both the ordinary cast (your first approach), and the reflective cast (your third approach) are checked. However, the reflective cast will not work with parametrized types (List<String>.class
doesn't compile ...).
The simplest and most flexible safe solution therefore is the ordinary cast.
Upvotes: 3
Reputation: 49572
Because all options involve a type cast they are all somehow "insecure" and can fail with ClassCastExceptions
.
I would definitely recommend to use helper methods like getString()
, getDate()
for common types that are often stored within this object. These methods are useful with all three options because the reduce the code the user of the object has to write.
But you still need a way to receive "uncommon" types from your object. For this I would go with the explicit cast or the class cast. The reason for this is that I think that these two ways are the ones that are mostly used. Even if the generic method call would work fine I think method calls like obj.genericGet<String>("myproperty");
are something not every java developers is aware of. It is rarely seen in practice.
Personally is would add an getObject()
method to the helper methods even if I don't like to move the type cast to the user of the object. The advantage of this is that you would have a consistent interface and this is what I would expect if I see methods like getString()
or getDate()
.
Upvotes: 1
Reputation: 120848
Personally, keeping lots of different objects in a single place and then checking what you want to return seems a bit wrong. May be you can store Holders in that BadlyCreatedClass.
Something like:
class Holder {
private long id;
private String name;
private Date dateofBirth;
//getters and setters
}
Then retrieve based on id, no casting thus required.
You could also tell us what you are trying to do.
Upvotes: 3
Reputation: 12538
As you already mentioned all of the above approaches are dangerous and may lead to ClassCastException
s during runtime.
If it is really necessary I'd prefer the 'generic cast' approach as it makes the interface explicit and self-expanatory (make the genericGet
private in this case). Of course you have to create boilderplate code for each class
in your container. So the advantage of 'Class.cast' is that you do not need those boilerplate methods.
Conclusion: If there is a well-defined number of classes in the container I'd go with 'generic cast'. In case you are required to support an endless number of classes I'd go with ' Class.cast'
Update: 'Explicit Cast' does have one advantage - the caller (user of the container) gets a reminder that there is a class-cast risk!
Just an opinion...
Upvotes: 1
Reputation: 9206
I would prefer to use generic cast. Why?
The explict cast is always more difficult to maintain. When you read the code you simply don't know what this method might return. What is more, the probability that the method will be used in wrong way and some ClassCastException
will ocurr during runtimme is quite high.
Class cast is even more difficult to maintain. In my opinion you create in this way something which might be called as spaghetti code.
When you create methods like getString
and getDate
you give very clear interface to your class. What is more it's always possible to get objects of other classes than String
and Date
because you provide also the generic method.
Upvotes: 1
Reputation: 8587
To give a quick answer, without going in deep about good-programming-practice...
I would use:
private <X> X genericGet(String property) {
}
public String getString(String property) {
//... checks on property (String specific)...
Object obj = genericGet(property);
//... checks if obj is what is expected and if good return it
return obj;
}
public Date getDate(String property) {
//... checks on property (Date specific)...
Object obj = genericGet(property);
//... checks if obj is what is expected and if good return it
return obj
}
make notice to the private genericGet. This way I can check if the get property is what I m waiting to receive and handle it in a correct way.
I could add checks in the getString depends on the property to make sure that the answer will be a String object.
I could do other checks for getDate in property to make sure it will be a date as will be returned.
Etc...
Upvotes: 3