[Qgis-developer] Suppress emitting of signal: Bad practice?

[Martin Dobias]

Hi Matthias

On Fri, Apr 19, 2013 at 4:07 PM, Matthias Kuhn <matthias.kuhn at gmx.ch> wrote:
> The QgsMapLayerRegistry::removeMapLayers allows to specify a parameter,
> which will suppress emitting signals (altough it seems that this is not
> even properly working [1]).
>
> This parameter can lead to strange behavior, where some parts of the
> software might not realize, that a layer has been removed and keep
> references to no longer available objects.

I completely agree with you - it is a bad practice to pass a parameter
whether to emit signal or not - it is typically something that leads
to wrong design.


> So my questions are:
> * Does somebody know why removeMapLayer(s) accepts this parameter?
> * Do you see any negative effect of removing these or any upside in
> keeping these?

I remember we have added that to support georeferencer. The workflow
with layers in QGIS is currently:
1. create Qgs[Vector/Raster]Layer instance
2. add the layer to map registry
3. map registry emits signal, caught by legend widget
4. legend widget adds the layer to the list of layers and updates the
list of layers in canvas
Georeferencer comes with its own map canvas and its own set of layers.
When we add a layer that should be georeferenced, we do not want it to
appear also on main canvas - that's where the 'emit signal' parameter
comes handy. Setting it to false will avoid loading the layer in the
main window.

There are surely more ways how to fix the problem, but probably all of
them involve slight redesign of how the components are interconnected.
Maybe we do not need a map layer registry. Or maybe we could have
several map layer registries (instead of just having singleton), one
for each canvas in the application. Or maybe we need something more
general than just a list of layers: e.g. right now the ordering is
defined by legend widget... for a proper logic/gui separation we
should keep the layers + ordering + grouping in a data structure (and
only let legend widget serve as its view and controller).

I am not sure how feasible is to do something about it now, but in any
case I would warmly welcome another shift of API freeze, the amount of
things we need to fix in API is quite high (e.g. in my pipeline:
symbology-ng "V2" renaming, feature iterators + requests API fixes,
QgsVectorLayer API updates, QgsExpression API fixes, sip API 2 and
probably more).

Cheers
Martin