[Qgis-developer] What will be in QGIS 2.0

[Radim Blazek]

On Thu, Jul 12, 2012 at 10:12 AM, Martin Dobias <wonder.sk at gmail.com> wrote:
>> I have doubts for example, if the quantity of circular dependencies is
>> good. See for example
>> http://qgis.org/api/classQgsMapCanvas.html
>
> Personally, I do not see a problem in this particular instance, do
> you? The only circular dependencies shown there are between
> QgsMapCanvas and QgsMapTool / QgsMapCanvasMap / QgsMapOverviewCanvas -
> and those are fine because they need to communicate with QgsMapCanvas.

As I said, I have doubts, I am not sure if it is a problem or how it
is important. I remember,  that once I was looking for something
around QgsMapCanvas and I found it quite difficult due to the circular
dependencies. I think that it is especially a problem for readability,
which I consider very important in OS project.

For example, QgsMapCanvasMap is using QgsMapCanvas in 2 methods to get
  QgsMapRenderer (QgsMapCanvas member) and call its methods. When
reading QgsMapCanvasMap source, you don't see that calling
QgsMapCanvasMap methods modifies QgsMapRenderer:

void QgsMapCanvasMap::resize( QSize size ) {
  ...
  mCanvas->mapRenderer()->setOutputSize( size, mPixmap.logicalDpiX() );
}

it is used in QgsMapCanvas:

  mMap->resize( size() );

I would prefer to set QgsMapRenderer size in QgsMapCanvas, either
simply something like

  mMap->resize( size() );
  mMapRenderer->setOutputSize( mMap->size() );

or with a resize signal from QgsMapCanvasMap.

The second method is

void QgsMapCanvasMap::render() {
  ...
  mCanvas->mapRenderer()->render( &paint );
}

which may be simply changed to

void QgsMapCanvasMap::render( QgsMapRenderer*  renderer) {
  ...
  renderer->render( &paint );
}

We would have one circular dependency less with just 2 simple changes.
In this case I don't see any real reason for circular dependency, it
does not seem to be intentional design, it seems that this
constellation appeared somehow itself during code evolution.

I believe that 2.0 API break is good opportunity to review such places.

Radim