tags:

views:

177

answers:

1

I am surely missing something huge, I've seen this problem several times so I would like to know your opinion on resolving these code-smells.

  • Background is a windows forms app organized using the MVC architecture pattern.
  • Data is organized in a hierarchical way (tree model), with different types of nodes representing different types of data.
  • Data view is a composite view, containing various views which can be added or modified according to customers request. Each data node can be represented using several views (table, chart, custom stuff), and each view knows how to display one or more different data node types.
  • Before displaying, some views also require that the data is processed in some way. Multiple views can use the same processed data, but not all views use the same data, so this processing is not related to the only one view or one node type.

So the thing that bothers me is that, in current implementation, nodes' interface is becoming overloaded with many properties like "Node.CanProvideThisData" or "Node.CanProvideSomeOtherData", which each derived node is required to implement. This shows that a node's interface knows too much about everything, right?

Who should, in your opinion, decide where and when will this processing happen, so that each view gets the desired processed data, and that no processing is duplicated?

Did you have similar problems before? Or do you think that there is something wrong with the implementation (or my understanding of implementation) of MVC? Or is this something that could be fixed with some refactoring?

Edit: One more thing that should be considered, of course, is handling later modifications. This interface, as bad as it may be, does force the programmer to implement each part of the contract, so any new node implementations should work instantly.

+1  A: 

Without a deeper understanding of your architecture, I can't be sure that my suggestion is a good one, but one possibility might be the use of Custom Attributes to declaratively state what data each node type can expose. Note that I'm assuming your "Can provide this data" methods are all very dumb and basically return a constant value for a particular type.

Your question states that your software uses polymorphism extensively and that you want to avoid having to re-implement multiple "can provide this sort of data" functions for each new type. In this case, consider adding a custom attribute ("AvailableDataAttribute") that explicitly states, "This type supports these sorts of data." The SortsOfData would probably be a flags enum, or something similar. You could then add AvailableDataAttribute to all the appropriate nodes. Next, you could remove implementations of your "can provide data X, can provide data Y" methods and replace them with a single method on the base class that accepts an instance of SortsOfData and evaluates whether its dynamic type claims to support that data in its attribute. It would just grab that type's SortsOfData value and check it for the desired value.

This has the big advantage of removing dynamic determination of static metadata about your node types (assuming that it is static metadata), and so your design is reflected more clearly in your types.

Of course, none of this holds if a particular node can support data at one point in time, then a few seconds later might not.

Greg D
Thanks! At present, a node either supports the data or it doesn't, but I guess I can easily make that "single base method" virtual if I need to add more logic to it later.
Groo