Paste P254

Get rid of RedEyeReductionTool mStatus
ActivePublic

Authored by muhlenpfordt on Aug 23 2018, 8:34 AM.
diff --git a/lib/redeyereduction/redeyereductiontool.cpp b/lib/redeyereduction/redeyereductiontool.cpp
index 557df77b..f8455b60 100644
--- a/lib/redeyereduction/redeyereductiontool.cpp
+++ b/lib/redeyereduction/redeyereductiontool.cpp
@@ -63,12 +63,16 @@ struct RedEyeReductionWidget : public QWidget, public Ui_RedEyeReductionWidget
{
stackedWidget->setCurrentWidget(mainPage);
}
+
+ bool adjusting() const
+ {
+ return stackedWidget->currentWidget() == mainPage;
+ }
};
struct RedEyeReductionToolPrivate
{
RedEyeReductionTool* q;
- RedEyeReductionTool::Status mStatus;
QPointF mCenter;
int mDiameter;
RedEyeReductionWidget* mToolWidget;
@@ -89,7 +93,7 @@ struct RedEyeReductionToolPrivate
QRectF rectF() const
{
- if (mStatus == RedEyeReductionTool::NotSet) {
+ if (!mToolWidget->adjusting()) {
return QRectF();
}
return QRectF(mCenter.x() - mDiameter / 2, mCenter.y() - mDiameter / 2, mDiameter, mDiameter);
@@ -102,7 +106,6 @@ RedEyeReductionTool::RedEyeReductionTool(RasterImageView* view)
{
d->q = this;
d->mDiameter = GwenviewConfig::redEyeReductionDiameter();
- d->mStatus = NotSet;
d->setupToolWidget();
view->document()->startLoadingFullImage();
@@ -117,7 +120,7 @@ RedEyeReductionTool::~RedEyeReductionTool()
void RedEyeReductionTool::paint(QPainter* painter)
{
- if (d->mStatus == NotSet) {
+ if (!d->mToolWidget->adjusting()) {
return;
}
QRectF docRectF = d->rectF();
@@ -145,10 +148,9 @@ void RedEyeReductionTool::mousePressEvent(QGraphicsSceneMouseEvent* event)
return;
}
event->accept();
- if (d->mStatus == NotSet) {
+ if (!d->mToolWidget->adjusting()) {
d->mToolWidget->diameterSpinBox->setValue(d->mDiameter);
d->mToolWidget->showMainPage();
- d->mStatus = Adjusting;
}
d->mCenter = imageView()->mapToImage(event->pos());
imageView()->update();
@@ -183,7 +185,7 @@ void RedEyeReductionTool::mouseDoubleClickEvent(QGraphicsSceneMouseEvent* event)
void RedEyeReductionTool::keyPressEvent(QKeyEvent* event)
{
QDialogButtonBox *buttons;
- if (d->mStatus == Adjusting) {
+ if (d->mToolWidget->adjusting()) {
buttons = d->mToolWidget->mainDialogButtonBox;
} else {
buttons = d->mToolWidget->helpDialogButtonBox;
@@ -225,7 +227,6 @@ void RedEyeReductionTool::slotApplyClicked()
RedEyeReductionImageOperation* op = new RedEyeReductionImageOperation(docRectF);
emit imageOperationRequested(op);
- d->mStatus = NotSet;
d->mToolWidget->showNotSetPage();
}
diff --git a/lib/redeyereduction/redeyereductiontool.h b/lib/redeyereduction/redeyereductiontool.h
index 3dfe8850..d7a5b9cf 100644
--- a/lib/redeyereduction/redeyereductiontool.h
+++ b/lib/redeyereduction/redeyereductiontool.h
@@ -41,11 +41,6 @@ class GWENVIEWLIB_EXPORT RedEyeReductionTool : public AbstractRasterImageViewToo
{
Q_OBJECT
public:
- enum Status {
- NotSet,
- Adjusting
- };
-
explicit RedEyeReductionTool(RasterImageView* parent);
~RedEyeReductionTool() override;
muhlenpfordt created this object in space S1 KDE Community.
rkflx added a subscriber: rkflx.Aug 24 2018, 10:42 PM

Should we get rid of mStatus and just use stackedWidget->currentWidget()?
Something like this: P254 (diff against this patch).

Sure, why not (*). 018f3d1b7bb4 was an improvement already, now you go one step further and eliminate a potentially out-of-sync variable, taking advantage of the boolean nature of the set of states.

Feel free to commit, patch LGTM. Given that we have isHidden, isVisible, isEnabled, isFullScreen etc, perhaps it would make sense to rename to isAdjusting?

(*) Let's assume the "cleanup" vs. "never touch a running system" consideration balances out in our favour here…