Fwd: Re: [PATCH] Separate download code sharable between desktop & mobile

[Willem Ferguson]

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
> somehow...
>
>
> 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
>
>
> /D
> _____
Kind regards,
willem