During my life as a programmer I have more and more come to dislike switch/case statements. They tend to be hard to grasp and with languages like C/C++ they are often the source of hard-to-find errors. Compilers that have warnings about missing default statements or missing cases for enumerated values can help to mitigate the situation, but still, I try to avoid them whenever I can.
The same holds true for if-elseif cascades or lots of if-elses in one method. They are hard to read, hard to maintain, increase the Crap, etc.
If you share this kind of mindset I invite you implement to some custom models with Qt4’s Model/View API. The design of the Model/View classes is derived from the well-known MVC pattern which separates data (model), presentation (view) and application logic (controller). In Qt’s case, view and controller are combined, supposedly making it simpler to use.
The basic idea of Qt’s implementation of its Model/View design is that views communicate with models using so-called model indexes. Using a table as an example, a row/column pair of (3,4) would be a model index pointing to data element in row 3, column 4. When a view is to be displayed it asks the attached model for all sorts of information about the data.
There are a few model implementations for standard tasks like simple string lists (QStringListModel) or file system manipulation (QDirModel < Qt4.4, QFileSystemModel >= Qt4.4). But usually you have to roll your own. For that, you have to subclass one of the abstract model classes that suits your needs best and implement some crucial methods.
For example, model methods rowCount and columnCount are called by the view to obtain the range of data it has to display. It then uses, among others, the data method to query all the stuff it needs to display the data items. The data method has the following signature:
QVariant data ( const QModelIndex& index, int role ) const
Seems easy to understand: parameter index determines the data item to display and with QVariant as return type it is possible to return a wide range of data types. Parameter role is used to query different aspects of the data items. Apart from Qt::DisplayRole, which usually triggers the model to return some text, there are quite a lot other roles. Let’s look at a few examples:
- Qt::ToolTipRole can be used to define a tool tip about the data item
- Qt::FontRole can be use to define specific fonts
- Qt::BackgroundRole and Qt::ForegroundRole can be used to set corresponding colors
So the views call data repeatedly with all the different roles and your model implementation is supposed to handle those different calls correctly. Say you implement a table model with some rows and columns. The design of the data method is forcing you into something like this …
QVariant data ( const QModelIndex& index, int role ) const { if (!index.isValid()) { return QVariant(); } switch (role) { case Qt::DisplayRole: switch (index.column()) { case 0: // return display data for column 0 break; case 1: // return display data for column 1 break; ... } break; case Qt::ToolTipRole: switch (index.column()) { case 0: // return tool tip data for column 0 break; case 1: // return tool tip data for column 1 break; ... } break; ... } }
… or equivalent if-else structures. What happens here? The design of the data method forces the implementation to “switch” over role and column in one method. But nested switch/case statements? AARGH!! With our mindset outlined in the beginning this is clearly unacceptable.
So what to do? Well, to tell the truth, I’m still working on the best™ solution to that but, anyway, here is a first easy improvement: handler methods. Define handler methods for each role you want to support and store them in a map. Like so:
#include <QAbstractTableModel> class MyTableModel : public QAbstractTableModel { Q_OBJECT typedef QVariant (MyTableModel::*RoleHandler) (const QModelIndex& idx) const; typedef std::map<int, RoleHandler> RoleHandlerMap; public: enum Columns { NAME_COLUMN = 0, ADDRESS_COLUMN }; MyTableModel() { m_roleHandlerMap[Qt::DisplayRole] = &MyTableModel::displayRoleHandler; m_roleHandlerMap[Qt::ToolTipRole] = &MyTableModel::tooltipRoleHandler; } QVariant displayRoleHandler(const QModelIndex& idx) const { switch (idx.column()) { case NAME_COLUMN: // return name data break; case ADDRESS_COLUMN: // return address data break; default: Q_ASSERT(!"Invalid column"); break; } return QVariant(); } QVariant tooltipRoleHandler(const QModelIndex& idx) const { ... } QVariant data(const QModelIndex& idx, int role) const { // omitted: check for invalid model index if (m_roleHandlerMap.count(role) == 0) { return QVariant(); } RoleHandler roleHandler = (*m_roleHandlerMap.find(role)).second; return (this->*roleHandler)(idx); } private: RoleHandlerMap m_roleHandlerMap; };
The advantage of this approach is that the supported roles are very well communicated. We still have to switch over the columns, though.
I’m currently working on a better solution which splits the data calls up into more meaningful methods and kind of binds the columns to specific parts of the data items in order to get a more row-centric approach: one row = one element, columns = element attributes. I hope this will get me out of this switch/case/if/else nightmare.
What do you think about it? I mean, is it just me, or is an API that forces you into crappy code just not so well done?
How would you solve this?
I love Qt, but yeah, after implementing a few QTableModel and QTreeModel backends I tend to prefer just going with the Item-based widgets now. Unless they’re going to be big lists. It’s definitely my least favourite part of the library.
I’ve done the converse of your approach; left the outermost switch or elseif chain intact, and delegated which-column-should-be-which-source-of-data to a big static table with function pointers and other info (like column name, resize mode, etc.).
Qt is usually nice, but you are true to remind us that it may sometime be far less than fine.
I’d do something like what you did too – but I’d put some marshaller somewhere in order to build the correct parameter list for my methods from the Qt parameters. This is similar to a design I am used to advocate to handle message passing (see http://www.gamedev.net/community/forums/mod/journal/journal.asp?jn=288850&reply_id=2312508 and the followup http://www.gamedev.net/community/forums/mod/journal/journal.asp?jn=288850&reply_id=2324298).