is it worth adding a class to distinguish types

[Don Kiddick]

Hi,
I'm working on a fairly typical client system which reads some data from a db, displays it in certain ways and allows users to edit some of that data in certain ways.
Some of the entities we load shares the same characteristics, that is, they consists of an id and a description. They are basically enumerated types, stored in the database.
For some context an example type is Region, which has two instances { 1, "America" } and { 2, "Europe" }
Another is Status, which has 3 instances, {1, "Pending"}, {2,"Closed"} and {3, "Open"}
Currently for each of these types, we have a Translator (to translate the entity between xml and back again), a Map (to map entity to id and back again) and the entity itself which is simply an immutable class with a description attribute.
We wanted to factor out the common functionality. One way is to create one SimpleType class that can be used to represent all of these entities. However we are worried this will reduce the typesafety of our application. For instance it could be easy to pass in a list of regions to a method that required a list of status.
So we're thinking of creating the SimpleType and then wrapping or extending it for each of these different types. e.g. we have a Region class and Status class that either wrap or extend SimpleType.
This however means that we are creating lots of decorators/subclasses that do not anything different to the classes they decorate/subclass. My nose is picking up a code smell, but I can't think of a better solution.
What do you think ?
Don.

[Ilja Preuss]

Originally posted by Don Kiddick:
We wanted to factor out the common functionality. One way is to create one SimpleType class that can be used to represent all of these entities. However we are worried this will reduce the typesafety of our application. For instance it could be easy to pass in a list of regions to a method that required a list of status.

How likely is that to happen? What would be the consequences?
Can you rely on other means than the compiler to reduce the likeliness of such problems - a good naming scheme, unit- or acceptance tests?

[Don Kiddick]

Originally posted by Ilja Preuss:

How likely is that to happen? What would be the consequences?

Reasonably likely. The consequences would be that we would see strange behaviour in the gui. Saving one value, another value being populated in the db. These bugs would be quite hard to find.

Originally posted by Ilja Preuss:

Can you rely on other means than the compiler to reduce the likeliness of such problems - a good naming scheme, unit- or acceptance tests?

We could. However typesafety is a good thing and something that makes a program more robust. However from your response, I presume the is no easy way to do this without introducing lots of similar classes that vary only in the the fact that thy have different names.
thanks,
D.

[Ilja Preuss]

Originally posted by Don Kiddick:
The consequences would be that we would see strange behaviour in the gui. Saving one value, another value being populated in the db. These bugs would be quite hard to find.

I don't yet fully follow you - how could a value being populated in the db you didn't save? Can you give a practical example, please?

We could. However typesafety is a good thing and something that makes a program more robust.

To some small amount, yes. It also makes the code more verbose and often more complex and inflexible. There probably is a balance you need to find.

However from your response, I presume the is no easy way to do this without introducing lots of similar classes that vary only in the the fact that thy have different names.

At least I don't know of a way, sorry. Seems to me as if to get type safety, you had to introduce different types the compiler can check against...

[Don Kiddick]

Originally posted by Ilja Preuss:

I don't yet fully follow you - how could a value being populated in the db you didn't save? Can you give a practical example, please?

Your issue object has a status and a region. Status and region are both the simple {id, description} objects I describe. By mixing these up in your save routine, you end up using the region id when you should be saving status id and vice-versa.
Does that make sense ?

To some small amount, yes. It also makes the code more verbose and often more complex and inflexible. There probably is a balance you need to find.

This is what I'm coming to realise. Thanks for the help !
D.

[Ilja Preuss]

Originally posted by Don Kiddick:
Your issue object has a status and a region. Status and region are both the simple {id, description} objects I describe. By mixing these up in your save routine, you end up using the region id when you should be saving status id and vice-versa.
Does that make sense ?

Ah - so you are only saving the id into the DB, and the same id means different things in different tables, correct?
One idea would be to make the id table-sensitive in your object model. You could simply make the id a String like "region_1", or make the id a tuple itself, perhaps using a typesafe enum (TYPE.REGION, 1). Your persistence layer would then provide the mapping between object id and db id + table, and could do some consistency checking (of course only at runtime, but that's better than nothing).
How does that sound to you?

[S Manch]

there is something called Generics in JAVA 1.5 that will help you do this...

for more info. I had a post earlier titled parameterized classes or try this below url that talks about it.


http://www.jguru.com/faq/view.jsp?EID=29392

[Ilja Preuss]

Originally posted by S Manch:
there is something called Generics in JAVA 1.5 that will help you do this...

You would still have to introduce a class for every type (table) you want to support, wouldn't you?

[Tim West]

Don,
Was your original idea to subclass SimpleType for each type (Region, Status etc)? That's the way I'd approach this...I'd say that the benefits outweigh the consequences. I'd wager that a large application the overhead will be negligible.
To enforce safety, I'd make each subtype have a private constructor (so no-one can create new instances), and have it store static instances of the actual regions or statuses. That way your code guarantees that you only have valid statuses, and that type-safety you talked about earlier. You'd get access to the statuses with Region.AMERICA or something.
This probably sounds a lot like the solution you're trying to avoid, but it still sounds pretty good I'd have thought...
Cheers,

--Tim

[Pho Tek]

Don,
I'm facing the same problem as yourself. Beside the type safety issue (which you can easily circumvent with enums), the other issue is that you need to be concerned about reporting. Since you're just storing 'Value types', your reporting tool needs to be able translate the value "1" back to "Status". You're are doing the dereferencing by storing them in XML, I went the way of creating enums which are persisted to their own tables.
Pho
[ April 07, 2004: Message edited by: Pho Tek ]

[Don Kiddick]

Tim, that's pretty much what I went with. Pho, yep, I have the same problem (with the mapping). If it's any help, this is the solution I cam up with.
My "SimpleObject" base class :

 
import java.util.HashMap;
import java.util.Map;
public class SimpleObject {
    
    //Maps class -> id -> SimpleObject
    private final static Map OBJECT_MAP = new HashMap();
    
    //Maps class -> SimpleObject -> id
    private final static Map ID_MAP = new HashMap();
    
    /** Holds value of property description. */
    private final String description;
    
    private final String id;
    
    /** Creates a new instance of SimpleObject */
    protected SimpleObject(String id, String description) {
        this.id = id;
        this.description = description;
        Map typeMap = getMap( OBJECT_MAP, getClass() );
        Map idMap = getMap( ID_MAP, getClass() );
        if( typeMap.put( id, this ) != null || idMap.put( this, id ) != null ) {
            throw new IllegalArgumentException( "Duplicate description: " + 
                description + " or id: " + id );
        }
    }
        
    private static Map getMap( Map classMap, Class theClass ) {
        Map map = (Map) classMap.get( theClass );
        if( map == null ) {
            map = new HashMap();
            classMap.put( theClass, map );
        }
        return map;
    }
    
    public static String getId( SimpleObject object ) {
        Map map = getMap( ID_MAP, object.getClass() );
        return (String) map.get( object );
    }
    
    protected static SimpleObject getSimpleObject( Class theClass, String id ) {
        Map map = getMap( OBJECT_MAP, theClass );
        return (SimpleObject) map.get( id );
    }
    
    /** Getter for property description.
     * @return Value of property description.*/
    public String getDescription() {
        return this.description;
    }
    
    public String toString()  {
        return getDescription();
    }
    
    public boolean equals( Object obj ) {
        if( obj == null || ( ! obj.getClass().equals( getClass() ) ) ) {
            return false;
        }
        SimpleObject so = (SimpleObject) obj;
        return so.id.equals( this.id );
    }
    
    public int hashCode() {
        return this.id.hashCode();
    }
    
}

An example of a concrete subclass :

 
import com.barcap.bookbuildertrader.domain.simpledata.SimpleObject;
import java.util.HashMap;
import java.util.Map;
public class LimitType extends SimpleObject {    
    
    /** Creates a new instance of LimitType */
    public LimitType(String id, String description) {
        super( id, description );
    }
    
    public static LimitType getLimitType(String id) {
        return (LimitType) getSimpleObject(LimitType.class, id );
    }
    
}

D.
[ April 08, 2004: Message edited by: Don Kiddick ]