On 19. 11. 2012 Edward Hades Toroshchin wrote:
> On Sun, Nov 18, 2012 at 10:43:07PM -0000, Matěj Laitl wrote:
> > [wrt BlockingQueryMaker created from code in
> > StatSyncing::CollectionProvider]
> >
> > Cannot be done. There couldn't be any "BlockingQueryMaker only created
> > in the main thread" restriction (which would make it useless) and
> > without it, the code wouldn't work.
>
> You have this restriction in StatSync::Provider, and according to
> yourself. And it won't make it useless. Again, your Provider is in main
> thread, but it isn't useless, despite that it's essentially a blocking
> interface to QueryMaker.
Yes. The StatSyncing::Provider has a "privilege" of the additional constraint
that it needs to be created in the main thread. The constraint is somewhat
unnatural and you've correctly questioned its purpose earlier in the review.
But given that StatSyncing::Providers are long-lived objects currently
exclusively created by StatSyncing::Controller, this constraint is not really
restrictive.
Now let's sketch what we would expect from a class named BlockingQueryMaker. I
would like to use it like this:
class MyBatchJob : public ThreadWeaver::Job { ... ]
MyBatchJob::run()
{
(...)
BlockinqQueryMaker qm( m_collection /* or whatever */ );
qm.addFilter( bla bla bla );
Meta::TrackList foundTracks = qm.run(); // can block for longer time
// do something with trackList...
}
^^^^^^
Would be impossible if the same constraint is applied to BlockingQueryMaker.
Okay, no problem creating BlockingQueryMaker in MyBatchJob constructor, they
say?
class MyBatchJob2 : public ThreadWeaver::Job
{
private:
BlockingQueryMaker m_qm;
}
MyBatchJob2::run()
{
Collection *coll = getCollectionToWorkOnFromSomewhere();
m_qm.setCollection( coll );
m_qm.addFilter( bla bla bla );
Meta::TrackList foundTracks = m_qm.run(); // can block for longer time
// do something with trackList...
}
^^^^^^
Still impossible with the current StatSyncing::Provider code, because setting
collection while already in non-main tread cannot be done.
This is why I'm reluctant to author BlockingQueryMaker, as it would have
either a sloppy API (with restrictive constraints) or would be tricky to
implement correctly *in my humble opinion*.
On the other hand, if someone else (you?) wants to create it (perhaps reusing
some StatSyncing::Provider code) I'm happy to review the code and use it
instead of ad-hoc code in Provider once it proves working correctly.
P.S.: Edward, when you've already compiled the StatSyncing code, perhaps you
can lightly test it? ;) UI in Amarok Config, scrobbling still working,
synchronizing with Last.fm (warning: non-interactive part is time-consuming)
or with an iPod...
Cheers,
Matěj
_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
