How should I refactor switch statements like this (Switching on type) to be more OO?
- by Taytay
I'm seeing some code like this in our code base, and want to refactor it:
(Typescript psuedocode follows):
class EntityManager{
private findEntityForServerObject(entityType:string, serverObject:any):IEntity {
var existingEntity:IEntity = null;
switch(entityType) {
case Types.UserSetting:
existingEntity = this.getUserSettingByUserIdAndSettingName(serverObject.user_id, serverObject.setting_name);
break;
case Types.Bar:
existingEntity = this.getBarByUserIdAndId(serverObject.user_id, serverObject.id);
break;
//Lots more case statements here...
}
return existingEntity;
}
}
The downsides of switching on type are self-explanatory. Normally, when switching behavior based on type, I try to push the behavior into subclasses so that I can reduce this to a single method call, and let polymorphism take care of the rest.
However, the following two things are giving me pause:
1) I don't want to couple the serverObject with the class that is storing all of these objects. It doesn't know where to look for entities of a certain type. And unfortunately, the identity of a type of ServerObject varies with the type of ServerObject. (So sometimes it's just an ID, other times it's a combination of an id and a uniquely identifying string, etc). And this behavior doesn't belong down there on those subclasses. It is the responsibility of the EntityManager and its delegates.
2) In this case, I can't modify the ServerObject classes since they're plain old data objects. It should be mentioned that I've got other instances of the above method that take a parameter like "IEntity" and proceed to do almost the same thing (but slightly modify the name of the methods they're calling to get the identity of the entity). So, we might have:
case Types.Bar:
existingEntity = this.getBarByUserIdAndId(entity.getUserId(), entity.getId());
break;
So in that case, I can change the entity interface and subclasses, but this isn't behavior that belongs in that class.
So, I think that points me to some sort of map. So eventually I will call:
private findEntityForServerObject(entityType:string, serverObject:any):IEntity {
return aMapOfSomeSort[entityType].findByServerObject(serverObject);
}
private findEntityForEntity(someEntity:IEntity):IEntity {
return aMapOfSomeSort[someEntity.entityType].findByEntity(someEntity);
}
Which means I need to register some sort of strategy classes/functions at runtime with this map. And again, I darn well better remember to register one for each my my types, or I'll get a runtime exception.
Is there a better way to refactor this? I feel like I'm missing something really obvious here.
