Skip to content

Commit

Permalink
[core] Address review: switch from string comparison to static const …
Browse files Browse the repository at this point in the history
…members
  • Loading branch information
gacarrillor authored and nyalldawson committed Oct 9, 2024
1 parent 4b02605 commit 4a73ef0
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 11 deletions.
2 changes: 2 additions & 0 deletions python/PyQt6/core/auto_generated/qgsdiagramrenderer.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,7 @@ Alters the size of rendered diagrams using a linear scaling.
#include "qgsdiagramrenderer.h"
%End
public:

QgsLinearlyInterpolatedDiagramRenderer();
~QgsLinearlyInterpolatedDiagramRenderer();
QgsLinearlyInterpolatedDiagramRenderer( const QgsLinearlyInterpolatedDiagramRenderer &other );
Expand Down Expand Up @@ -965,6 +966,7 @@ the rendered diagram is given by a combination of subrenderers.
#include "qgsdiagramrenderer.h"
%End
public:

QgsStackedDiagramRenderer();

virtual QgsStackedDiagramRenderer *clone() const /Factory/;
Expand Down
2 changes: 2 additions & 0 deletions python/core/auto_generated/qgsdiagramrenderer.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,7 @@ Alters the size of rendered diagrams using a linear scaling.
#include "qgsdiagramrenderer.h"
%End
public:

QgsLinearlyInterpolatedDiagramRenderer();
~QgsLinearlyInterpolatedDiagramRenderer();
QgsLinearlyInterpolatedDiagramRenderer( const QgsLinearlyInterpolatedDiagramRenderer &other );
Expand Down Expand Up @@ -965,6 +966,7 @@ the rendered diagram is given by a combination of subrenderers.
#include "qgsdiagramrenderer.h"
%End
public:

QgsStackedDiagramRenderer();

virtual QgsStackedDiagramRenderer *clone() const /Factory/;
Expand Down
8 changes: 7 additions & 1 deletion src/core/qgsdiagramrenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,8 @@ void QgsDiagramRenderer::_writeXml( QDomElement &rendererElem, QDomDocument &doc
rendererElem.setAttribute( QStringLiteral( "attributeLegend" ), mShowAttributeLegend );
}

const QString QgsSingleCategoryDiagramRenderer::DIAGRAM_RENDERER_NAME_SINGLE_CATEGORY = QStringLiteral( "SingleCategory" );

QgsSingleCategoryDiagramRenderer *QgsSingleCategoryDiagramRenderer::clone() const
{
return new QgsSingleCategoryDiagramRenderer( *this );
Expand Down Expand Up @@ -698,6 +700,8 @@ void QgsSingleCategoryDiagramRenderer::writeXml( QDomElement &layerElem, QDomDoc
layerElem.appendChild( rendererElem );
}

const QString QgsLinearlyInterpolatedDiagramRenderer::DIAGRAM_RENDERER_NAME_LINEARLY_INTERPOLATED = QLatin1String( "LinearlyInterpolated" );

QgsLinearlyInterpolatedDiagramRenderer::QgsLinearlyInterpolatedDiagramRenderer()
{
mInterpolationSettings.classificationAttributeIsExpression = false;
Expand Down Expand Up @@ -858,6 +862,8 @@ void QgsLinearlyInterpolatedDiagramRenderer::writeXml( QDomElement &layerElem, Q
layerElem.appendChild( rendererElem );
}

const QString QgsStackedDiagramRenderer::DIAGRAM_RENDERER_NAME_STACKED = QStringLiteral( "Stacked" );

QgsStackedDiagramRenderer *QgsStackedDiagramRenderer::clone() const
{
return new QgsStackedDiagramRenderer( *this );
Expand Down Expand Up @@ -925,7 +931,7 @@ void QgsStackedDiagramRenderer::renderDiagram( const QgsFeature &feature, QgsRen

for ( const auto &stackedRenderer : stackedRenderers )
{
if ( stackedRenderer->rendererName() == QStringLiteral( "Stacked" ) )
if ( stackedRenderer->rendererName() == QgsStackedDiagramRenderer::DIAGRAM_RENDERER_NAME_STACKED )
{
// Nested stacked diagrams will use this recursion
stackedRenderer->renderDiagram( feature, c, newPos, properties );
Expand Down
11 changes: 8 additions & 3 deletions src/core/qgsdiagramrenderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -874,12 +874,13 @@ class CORE_EXPORT QgsDiagramRenderer
class CORE_EXPORT QgsSingleCategoryDiagramRenderer : public QgsDiagramRenderer
{
public:
static const QString DIAGRAM_RENDERER_NAME_SINGLE_CATEGORY SIP_SKIP;

QgsSingleCategoryDiagramRenderer() = default;

QgsSingleCategoryDiagramRenderer *clone() const override SIP_FACTORY;

QString rendererName() const override { return QStringLiteral( "SingleCategory" ); }
QString rendererName() const override { return QgsSingleCategoryDiagramRenderer::DIAGRAM_RENDERER_NAME_SINGLE_CATEGORY; }

QList<QString> diagramAttributes() const override { return mSettings.categoryAttributes; }

Expand Down Expand Up @@ -909,6 +910,8 @@ class CORE_EXPORT QgsSingleCategoryDiagramRenderer : public QgsDiagramRenderer
class CORE_EXPORT QgsLinearlyInterpolatedDiagramRenderer : public QgsDiagramRenderer
{
public:
static const QString DIAGRAM_RENDERER_NAME_LINEARLY_INTERPOLATED SIP_SKIP;

QgsLinearlyInterpolatedDiagramRenderer();
~QgsLinearlyInterpolatedDiagramRenderer() override;
QgsLinearlyInterpolatedDiagramRenderer( const QgsLinearlyInterpolatedDiagramRenderer &other );
Expand All @@ -926,7 +929,7 @@ class CORE_EXPORT QgsLinearlyInterpolatedDiagramRenderer : public QgsDiagramRend

QSet< QString > referencedFields( const QgsExpressionContext &context = QgsExpressionContext() ) const override;

QString rendererName() const override { return QStringLiteral( "LinearlyInterpolated" ); }
QString rendererName() const override { return QgsLinearlyInterpolatedDiagramRenderer::DIAGRAM_RENDERER_NAME_LINEARLY_INTERPOLATED; }

void setLowerValue( double val ) { mInterpolationSettings.lowerValue = val; }
double lowerValue() const { return mInterpolationSettings.lowerValue; }
Expand Down Expand Up @@ -997,6 +1000,8 @@ class CORE_EXPORT QgsLinearlyInterpolatedDiagramRenderer : public QgsDiagramRend
class CORE_EXPORT QgsStackedDiagramRenderer : public QgsDiagramRenderer
{
public:
static const QString DIAGRAM_RENDERER_NAME_STACKED SIP_SKIP;

QgsStackedDiagramRenderer() = default;

QgsStackedDiagramRenderer *clone() const override SIP_FACTORY;
Expand All @@ -1018,7 +1023,7 @@ class CORE_EXPORT QgsStackedDiagramRenderer : public QgsDiagramRenderer

QList<QString> diagramAttributes() const override;

QString rendererName() const override { return QStringLiteral( "Stacked" ); }
QString rendererName() const override { return QgsStackedDiagramRenderer::DIAGRAM_RENDERER_NAME_STACKED; }

void readXml( const QDomElement &elem, const QgsReadWriteContext &context ) override;
void writeXml( QDomElement &layerElem, QDomDocument &doc, const QgsReadWriteContext &context ) const override;
Expand Down
4 changes: 2 additions & 2 deletions src/gui/vector/qgsdiagramproperties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ void QgsDiagramProperties::syncToRenderer( const QgsDiagramRenderer *dr )
else // already a diagram renderer present
{
//single category renderer or interpolated one?
if ( dr->rendererName() == QLatin1String( "SingleCategory" ) )
if ( dr->rendererName() == QgsSingleCategoryDiagramRenderer::DIAGRAM_RENDERER_NAME_SINGLE_CATEGORY )
{
mFixedSizeRadio->setChecked( true );
}
Expand Down Expand Up @@ -543,7 +543,7 @@ void QgsDiagramProperties::syncToRenderer( const QgsDiagramRenderer *dr )
}
}

if ( dr->rendererName() == QLatin1String( "LinearlyInterpolated" ) )
if ( dr->rendererName() == QgsLinearlyInterpolatedDiagramRenderer::DIAGRAM_RENDERER_NAME_LINEARLY_INTERPOLATED )
{
const QgsLinearlyInterpolatedDiagramRenderer *lidr = dynamic_cast<const QgsLinearlyInterpolatedDiagramRenderer *>( dr );
if ( lidr )
Expand Down
2 changes: 1 addition & 1 deletion src/gui/vector/qgsdiagramwidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void QgsDiagramWidget::syncToOwnLayer()
// pick the right mode from the layer
if ( dr && dr->diagram() )
{
if ( dr->rendererName() == QLatin1String( "Stacked" ) )
if ( dr->rendererName() == QgsStackedDiagramRenderer::DIAGRAM_RENDERER_NAME_STACKED )
{
mDiagramTypeComboBox->setCurrentIndex( ModeStacked );
}
Expand Down
8 changes: 4 additions & 4 deletions src/gui/vector/qgsstackeddiagramproperties.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ void QgsStackedDiagramProperties::syncToLayer()
mStackedDiagramSpacingSpinBox->setValue( settingList.at( 0 ).stackedDiagramSpacing() );
mStackedDiagramSpacingUnitComboBox->setUnit( settingList.at( 0 ).stackedDiagramSpacingUnit() );

if ( dr->rendererName() == QLatin1String( "Stacked" ) )
if ( dr->rendererName() == QgsStackedDiagramRenderer::DIAGRAM_RENDERER_NAME_STACKED )
{
const QgsStackedDiagramRenderer *stackedDiagramRenderer = static_cast< const QgsStackedDiagramRenderer * >( dr );
const auto renderers = stackedDiagramRenderer->renderers();
Expand Down Expand Up @@ -396,9 +396,9 @@ QVariant QgsStackedDiagramPropertiesModel::data( const QModelIndex &index, int r
}
else
{
if ( dr->rendererName() == QLatin1String( "SingleCategory" ) )
if ( dr->rendererName() == QgsSingleCategoryDiagramRenderer::DIAGRAM_RENDERER_NAME_SINGLE_CATEGORY )
return tr( "Fixed" );
else if ( dr->rendererName() == QLatin1String( "LinearlyInterpolated" ) )
else if ( dr->rendererName() == QgsLinearlyInterpolatedDiagramRenderer::DIAGRAM_RENDERER_NAME_LINEARLY_INTERPOLATED )
return tr( "Scaled" );
else
return tr( "Unknown" );
Expand Down Expand Up @@ -480,7 +480,7 @@ bool QgsStackedDiagramPropertiesModel::setData( const QModelIndex &index, const
QgsDiagramSettings ds = dr->diagramSettings().at( 0 );
ds.enabled = ( value.toInt() == Qt::Checked );

if ( dr->rendererName() == QLatin1String( "SingleCategory" ) )
if ( dr->rendererName() == QgsSingleCategoryDiagramRenderer::DIAGRAM_RENDERER_NAME_SINGLE_CATEGORY )
{
QgsSingleCategoryDiagramRenderer *dsr = static_cast< QgsSingleCategoryDiagramRenderer * >( dr );
dsr->setDiagramSettings( ds );
Expand Down

0 comments on commit 4a73ef0

Please sign in to comment.