Code review comment for lp:~osomon/unity-2d/dnd-reorder-apps

I addressed most of the points you raised above, and will shortly submit a new merge request to get those changes in. See my comments on the points I didn’t address or addressed only partially:

> try using QWidget::grabKey when the drag is started to get all
> mouse events to your window […]

As discussed and agreed, this won’t be implemented for now.

> Can you please add a short comment to explain why you need qMin and qMax ?

That looks pretty obvious to me, the pointers to the LauncherApplications are called firstApplication and secondApplication for a reason, an additional comment would just state the obvious.

> Can't we just change the LauncherApplication::priorty to be a double […]

We could, but priorities really are integers, so it’s better to keep our interfaces clean and work around the issues internally, in the implementation. On top of that this code is going to go away very soon, changing the interface could potentially introduce regressions for no real benefit.

> One option could be to make it somehow possible to ask the aggregator
> to tell us which model an item is in, and then do the move operation
> on that model directly.

That is exactly what I’m doing here: modelAtIndex(from) tells us which model an item is in and we invoke the move() method on this model.
The reason I need to do that the metaprogramming-way, as the comment states it, is that "move" is not a member of the QAbstractListModel interface. I could modify the ListAggregatorModel class to store instances of an interface that inherits QAbstractListModel and adds to it a "move" method, but that sounds a bit overkill, don’t you think?
The metaprogramming way is flexible enough that if the method doesn’t exist on the model, the call will simply be ignored.