Caret7:Development/DesignStrategies

From Van Essen Lab

Jump to: navigation, search

Design Patterns/Idioms for Maximum Flexibility

  1. We should move methods that don't require access to a class's private data outside of the class. This will allow us to increase encapsulation, and make future design changes much easier. It also helps to differentiate from methods that are crucial to the function of an object from those that operate ON that objects. There are two approaches:
    1. For each class, create another class with the same name and "Helper" appended as the suffix. Then add static only utility methods to these Helper methods. The rule of thumb is that if it doesn't need to access the internals of the class, it goes in the helper class. Another variant of this, if we insist on keeping these methods inside the same class, is to make them static, so that it's clear which methods work on private data, and which ones are there for convenience.
    2. Taking this idea further, we can create templates that work classes that have a certain interface. This would be nice to have, but even going with the first approach will be a big improvement.
      There are cases where the above model breaks down, such as operator overloading for classes, but for the most part, we should try to move utility operations outside the class, or in the very least, organize the layout of our code so that it's obvious which methods are essential to the operation of the class, and which methods are for convenience.
  2. We should avoid creating reach-through classes unless there is a compelling reason for them. One good example of a reach-through, or anemic class, is "Actions", located in GuiQT. Actions essentially is a dummy class, that processes events for MainWindow. My first reaction was wondering if moving this outside of MainWindow really saved us much, since MainWindow still needed to process the event, and know what to do with the event. It seemed to me that moving that last, often trivial, bit of behavior, the "Action" itself, to another class actually added to confusion, since it simply put the Action somehwere else, while requiring MainWindow to know everything else.
    This leads to an important point. Often times, when a class is anemic, it's not because we've over-modularized our code. The solution is not to simply get rid of the anemic class. More often that not, it's because we sense that something should be abstracted, but we get it wrong, and mistakenly highlight a trivial bit of functionality as the important behavior to abstract, and overlook the important bits of functionality that we actually should be modelling.
    A careful enumeration of all the things that happen when an event is triggered shows us:
    1. Event is sent from OS/QT/Windowing API to the App
    2. The App determines the type of event, and calls one or more listeners to process the event
    3. Listeners perform an Action when they are called by the App
    What we notice is that one concept that is appearing three times above, that is, "App". This implies that we haven't properly abstracted our event processing behavior , as there is no obvious place to put the knowledge of what to do with an event. Certainly making where we put it global (as the word "App" implies), or undefined, can lead to issues.
    This leads us to the conlusion that what we really want to abstract is the behavior of taking an event, and calling registered listeners. What we really need is a class of type "EventManager", that all listeners register with, and that all event sources register with. Then it's job is to map which events go to which listeners in a flexible, extensible way.
    Once we do this, we notice that the real role of Events and Actions, is to send some piece of information to EventManager for processing, and in the case of Actions, to process these events. This suggests that the proper role of the Actions base class (and Events) is not to actually process the event, but to be a base class that provides an interface that allows objects of type Action to be registered with EventManager.
  3. In general, don't do things that break down encapsulation. An object should have a single, clearly design responsibility. From Caret7:
    GiftiEndianEnum::GiftiEndianEnum(
    const Enum e,
    const int32_t integerCode,
    const AString& name,
    const AString& giftiName)
    {
    this->e = e;
    this->integerCode = integerCode;
    this->name = name;
    this->giftiName = giftiName;
    }
    /**
    * Destructor.
    */
    GiftiEndianEnum::~GiftiEndianEnum()
    {
    }
    void
    GiftiEndianEnum::initialize()
    {
    if (initializedFlag) {
    return;
    }
    initializedFlag = true;
    enumData.push_back(GiftiEndianEnum(ENDIAN_BIG, 0, "ENDIAN_BIG", "BigEndian"));
    enumData.push_back(GiftiEndianEnum(ENDIAN_LITTLE, 1, "ENDIAN_LITTLE", "LittleEndian"));
    }
    The problem with the code above is that the enum is actually implementing two very different things. The first thing it implements is a type that holds all the valid enums, and the next thing it implements is a container for each enum. The problem with having one class do both is that is breaks down any kind of encapsulation. What we end up with is a class where a single enum knows everything about the list that holds it, and the list that holds it knows everything about the implementation of the things that it contains.
    This same issue also shows up for CaretAssertion, and BrainOpenGL.
  4. An object should either be a singleton, or an instance, but not both. If you REALLY need an object to exist both as a singleton and as multiple instances (in the case of BrainModelOpenGL), then you should move the globally accessible parts of the class to a different class. Implementing and using an object both as a global singleton and a locally creatable class implies that you are coupling two very, very different functions into the same class, which can create quite a bit of added complexity later.
  5. Never allow for circular dependencies. From a design perspective they are almost always the wrong thing to do. In caret7, BrainStructure has a pointer to Brain, and Brain contains a list of BrainStructures. This indicates that there is inappropriate intimacy between the classes. The communication should be separated by a mediator class that manages communication between the two. Another example is that Actions have a handle to WindowMain, and WindowMain has a handle to actions. This type of coupling will cause problems in the future.
  6. Think carefully about what kinds of programming constructs are appropriate for the task at hand. Just because a certain style is good in one context doesn't mean it won't cause problems in others. Don't simply make everything an object, or make everything a template, etc. Just because something is good doesn't always mean that more is better. Here's an example. In QT the class QString has convenenience methods for converting from a char * to a QString. i.e. The following syntax works:
    char * mystring = "my string";
    QString temp = mystring;
    Counter-intuitively, the '=' operator is not communicative in their current implementation, meaning that the following DOES NOT work:
    QString temp = "my string";
    char * mystring= temp;
    Why doesn't it work? Is there some inherent limitation in C++ that prevents overloading the = operator so that it can convert from a QString to a char *? The reason it doesn't work is that the developers of QString mindlessly put the operator= methods INSIDE the class. By doing it this way, they limited themselves to only being able to write methods that could have the QString as an l-value. By mindlessly following object-oriented principles, without THINKING about why they were doing it, they created a serious design limitation.
    The workaround: Implement the operators as methods outside the class, this way they aren't limited to only being able to use an object as an l-value. This is just one of many examples where following conventional wisdom can cause problems.
  7. Think carefully about what an object's responsibilities are. If there are multiple responsibilities, try to break them up.
  8. Don't force abstract, generic behavior to be handled by objects whose purpose is very specific. Generic behavior should be abstracted and moved to a more general purpose object. i.e. Brain shouldn't handle coordinating events. Brain is a very specific, narrow, concrete thing, while managing events is a very general, reusable, global thing. BrainOpenGL makes it's own OpenGL drawing calls, which in many cases is generic (triangles are triangles), when it should probably be using a lower level class that handles drawing. Additionally, the thing that manages drawing for all objects within the scene shouldn't be prefixed with Brain, as we may want to draw other things in addition to the Brain. Instead, we should have a generic object for drawing all things within the scene, and BrainModelOpenGL could be one of the things it contains.

Design Patterns/Idioms for Maximum Performance

  1. We should attempt, whereever possible, to use idioms that maximize performance. One example is passing around groups of coordinates for processing:
    1. Good Performance, Bad Design -
      ProcessCoords(double *coords, int length); // This is good for performance because we can avoid extra function calls
    2. Bad Performance, Mediocre Design:
      for(int i = 0;i<length; i++)
      {
      double coord[3];
      GetCoord(coord);
      ProcessCoord(coord);
      }
      The above idiom is marginally better in terms of encapsulation, but terrible for performance because we have function call overhead every single time we want to process a single coord.
    3. Good Performance, Good Design -
      ProcessCoords(std::vector<double> &coords); //This avoids function call overhead while providing safe encapsulation of coordinates.
    4. There are probably even better idioms, but we can certainly do better than (2.2), which is used all over the place in Caret5
  2. We should use inline for trivial functions that are used in tight inner loops. For example, adding the inline keyword before every function in caret5's math library improved performance by 10% in my benchmarks.

Further Reading

A nice list of tips (some of which I hadn't seen before), for how to write modular, reusable code:

  1. Tips for maximizing reuseability of C++ code:
    http://proquest.safaribooksonline.com/book/programming/cplusplus/0321334876/designs-and-declarations/ch04lev1sec6
  2. Another good book to look at is this:
    http://proquest.safaribooksonline.com/book/programming/cplusplus/0201704315/techniques/ch01

While we have a lot of suggestions about text formatting and style, we have a dearth of suggestions for how we should approach designing our object model, and structuring our software. Hopefully this page will be a catch-all for other techniques that we decide are important.

Personal tools
Sums Database