mbasaglia / Qt-Color-Widgets

Color wheel widget and dialog for Qt
147 stars 54 forks source link

Common interface for color widgets #30

Closed caryoscelus closed 7 years ago

caryoscelus commented 7 years ago

Some widgets have already overlapping interface (such as setColor), but they have no common abstract class / interface and so the only way to call these functions from abstract pointer is to use Qt meta calls (QMetaObject::invokeMethod(widget, "setColor", Q_ARG(QColor, c));). It would be more convenient for color widgets to inherit a common interface class, which would also handle additional code like setHue etc.

The only downside of this that i can see is that it would make those common functions non-slots (because Qt doesn't allow multi-inheritance from QObject), but since Qt5 it's possible to use connect without slots (e.g. https://woboq.com/blog/new-signals-slots-syntax-in-qt5.html) so i would consider it as a worth trade-off.

mbasaglia commented 7 years ago

That might break designer and scripting integration though

caryoscelus commented 7 years ago

Really? I haven't worked with either, so i can't tell, but i see no reason why it should break anything, rather than perhaps ability to connect slots inside designer.

mbasaglia commented 7 years ago

Yes, that's what I meant by designer integration. To be honest this isn't something I would want to lose.

By the way, what do you need the base interface for?

caryoscelus commented 7 years ago

Firstly, to avoid code duplication. This includes both existing widgets and possibly new ones.

Secondly, to be able to sync widgets without going through them manually - right now i have to use Qt meta call for that.

mbasaglia commented 7 years ago

Can't you use slots to sync them?

caryoscelus commented 7 years ago

That's what i do with qt meta object now. And if you mean use connect - i tried that, but then there would be too much calls (if not infinite recursion); to avoid that i suppress signals before setting color.

mbasaglia commented 7 years ago

This would break existing functionality without providing many benefits so closing this.

caryoscelus commented 7 years ago

One option to keep functionality would be to put common methods into base class and have slot wrappers in widgets (possibly through macros, but i'm not sure moc supports this).

mbasaglia commented 7 years ago

The thing is that the base class won't really offer anything new and any workaround can only lead to hacks that are much harder to maintain or loss of functionality.

caryoscelus commented 7 years ago

Well, i'm already forced to use hacks because of current design.

As for maintaining, i disagree. Currently duplicated code that is hard to maintain may actually change (e.g. setColor has slightly different implementations for different widgets). If duplicated code would only consist of

public Q_SLOTS:
  void setColor(QColor const& c) { doSetColor(c); }

or even

public Q_SLOTS:
  SET_COLOR_SLOT

it would require no additional maintaining.