[Marble-devel] FileManager: simplify API

[Thibaut Gridel]

Hello Konrad,

On Saturday 19 January 2013, Konrad Enzensberger wrote:
> Hello Thibaut,
> 
> you made a change to FileManager:
> ***************************************************************************
> *************** FileManager: simplify API
> 
>     The goal is that:
>     - FileManager takes responsibility of the given fileName "key".
>     - FileLoader does search for an actual file on disk and launches
> parsing - GeoDataDocument knows of the actual file (and path) on disk.
> **************************************************************************
> ******************
> 
> I have a question about your modifications:

Well,
as said in the commit, the idea is to simplify the API. Maybe i've gone too 
far though.
The intent is to hide the internal details in order to concentrate on what the 
FileManager is about: 
- ask to add / remove files (or raw data),
- signal when they have been added/removed and
- retrieve a GeoDataDocument* with at()

I also moved away from index based storing, because the identifier from user 
pov is the "key" argument, so changed at() and signals accordingly.

> From FileManager::appendLoader(..) you removed this signal/slot connection:
> connect( loader, SIGNAL( newGeoDataDocumentAdded( GeoDataDocument* ) ),
> this, SLOT( addGeoDataDocument( GeoDataDocument* ) ) );
> 

To me the addGeoDataDocument( GeoDataDocument* ) is redundant as the runner 
just finished anyway, and holds a non-null document if it parsed something.
Therefore the method
void FileManagerPrivate::cleanupLoader( FileLoader* loader )
now handles the document together with cleaning the loader. I considered this 
threaded loader handling an "internal detail".

> Additional you removed the FileManager::addGeoDataDocument(...) slot.
> But in FileLoader::run() , the FileLoader emits the signal
> newGeoDataDocumentAdded() , which now is not connected to any other slot.
> 
> In my app is need this signal/slot connection when loading kml files, is
> there a special reason for removing this ?

You are right that newGeoDataDocumentAdded is useless with comment above, i 
should remove it.

To suit your needs, FileManager now emits :
void fileAdded( const QString &key );
when such file successfully got treated, and you can retrieve the file with 
GeoDataDocument *at( const QString &key );

Feeding the document to MarbleModel::treeModel() is done internally also.

Hope this is clearer. We're still early in Marble 1.6 development to smooth 
this in.

Also if all you want is to parse a file and get a result, you may use 
MarbleRunnerManager. It has a synchronous openFile and asynchronous parseFile 
and parsingFinished signals.
This class is independent from MarbleModel and so doesn't feed the treeModel.

If you have other usecases then please state them, we can have a look for 
those.

Best Regards,

Thibaut