[Marble-devel] Re: Review Request: Map Creation Wizard GeoScene Refactoring

[Bernhard Beschow]

> On 2010-12-28 17:17:14, Bernhard Beschow wrote:
> > trunk/KDE/kdeedu/marble/src/lib/MapWizard.cpp, line 751
> > <http://svn.reviewboard.kde.org/r/6228/diff/2/?file=43247#file43247line751>
> >
> >     Again, I'd initialize d->document here and get rid of MapWizard::handleDocumentAttributes().
> >     
> >     Perhaps it's even better to create it here as a temporary instance only and pass it as an argument to the dgml methods. That way, the flow of data is much cleaner, which is important for further development and maintainability.

BTW, removing MapWizard::handleDocumentAttributes() doesn't prevent us from introducing it at some later point. But for now, I can't see any immediate benefit of having it. Thus, in the spirit of agile development, I'd keep the design flexible until we feel any need for strengthening it.

Perhaps - in the future - base tiles and theme icons should be stored as pixmaps/images instead of paths. This way, the map wizard can provide visual feedback to the user early in the map creation process. However, images can't be stored in the GeoScene* classes directly, which implies that we need some other data fields in the map wizard anyway. That's why I (currently) prefer a solution where the GeoScene* classes are only used for generating the map structure, which is the last step in the process.


- Bernhard


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6228/#review9449
-----------------------------------------------------------


On 2010-12-28 16:31:39, Utku Aydin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6228/
> -----------------------------------------------------------
> 
> (Updated 2010-12-28 16:31:39)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> Currently the Map Creation Wizard is making use of QXmlStreamWriter and feeds bare hardcoded strings into the DGML file. That's the first step of refactoring the Map Creation Wizard in a way so that it works based on GeoScene classes which has benefits in terms of extensibility, maintenance, etc.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/src/lib/MapWizard.h 1209913 
>   trunk/KDE/kdeedu/marble/src/lib/MapWizard.cpp 1209913 
> 
> Diff: http://svn.reviewboard.kde.org/r/6228/diff
> 
> 
> Testing
> -------
> 
> At the moment, GeoSceneDocument provides texts of summary page's labels.
> 
> 
> Thanks,
> 
> Utku
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20101228/b0fa7de6/attachment.htm