Fwd: Re: [PATCH] Separate download code sharable between desktop & mobile
willemferguson at zoology.up.ac.za
Sun Jan 31 12:27:54 PST 2016
On 31/01/2016 20:36, Dirk Hohndel wrote:
> I'm not sure why you say that. The issue that I see is the filtering
> that's missing (i.e., just shoing the support DCs), but the models should
> work in QML with very little modification.
These comments are very valuable to me.
The object here was just to get the desktop version to work normally,
given the removal of some code.
Filtering is certainly a priority, but probably implementing another
method of the same class.
> That filename is confusing... based on reading what you are doing there
> below I don't think this is the "manager".
> Also, since it is supposed to be used both from desktop and mobile it
> would have to be either in subsurface-core or (with a little restructuring
> - as really it should hold only the models) in qt-models.
I thought it should go somewhere where both projects can easily see it.
Desktop-widgets is clearly not appropriate.
Same applies to qt-mobile. Maybe subsurface-core. let me just tell you
what my vision has been. That this new class
contains numerous methods (e.g. filtering of dc names, other
interactions with the downloadFromDCWidget, (there
are quite a few methods still to be added) as well as the interfacing
(effectively the models) for the QML. Would it
be advantageous to keep the models and the methods separate?
> Whitespace. You're indenting with four spaces instead of tab. You need to
> install the Subsurface settings in QtCreator would be my guess. Look for
> details how to do this in the CodingStyle document.
Apologies. I set QtCreator to use tabs and remove whitespace without
checking that it is actually doing this. Its
working ok now.
> So fill_computer_list() right now populates the vendorList which is part
> of the DownloadFromDCWidget... not sure how you can do this in a helper
> function that's part of a different class without passing that list around
> Ah, that's how you are doing it... I don't think that's a good way to do
> this. I'd create the model in the helper and pass that around instead of
> just a string list.
Will do that. That was part of my considerations during the process,
but I was not sure the existing models used by the desktop were very useful,
that is after looking at the code on the qt-models directory.
> I see that you took what must have seemed like a reasonable first step in
> abstracting this out by moving just the creation of the QStringList into a
> helper function. I keep thinking that we should abstract this out at the
> QAbstractListModel level...
Am I understanding you right?
The class (downloadmanager) should inherit from QAbstarctListObject
not from QObject? Makes sense.
We can talk about a name for this class? I was also only 95% happy
with that name.
> Create a simple white list in that model helper function where you can
> match the known supported dive computers and return an appropriate model.
> And then use that model either in the desktop UI or in the QML UI
More information about the subsurface