Code review comment for lp:~mixxxdevelopers/mixxx/iTunes2

Revision history for this message
RAFFI TEA (raffitea) wrote :

I think there are two options. (1) We follow RJ's proposal. However, we get a deeper class hierarchy, which is really not my favor. (2) We refactor BaseSqlTableModel. In particular, we move the connect statement in a separate method (e.g., setTrackChangeAwareness(bool) ). If the parameter is true, we connect the signal. This is probably the quickest way to realize.

Both suggestions are not the 'cleanest'. But I am more towards solution (2) because there's no real difference between native and non-native library features unless the latter are read-only models and trackChanged signals are unintended. This brings me to another idea: (3) We could disconnect the signal in BaseSqlTableModel::readOnlyFlags(QModelIndex ind). Basically, all non-native Mixxx library features return read-only model flags, e.g., iTunes models return BaseSqlTableModel::readOnlyFlags() in their flags() method.

What do you think RJ or Albert?

« Back to merge proposal