dynamic_cast considered annoying

Mr Flibble

unread,

Feb 4, 2016, 7:00:51 AM2/4/16

to

Check out my codes!

void radio_button::set_on_state(bool aOnState)
{
if (iOnState != aOnState)
{
if (aOnState)
{
i_widget* candidate = &link_after();
while (candidate != this)
{
if (is_sibling(*candidate))
{
// Teh ghastly dynamic_cast! A simpler CLEAN solution which
doesn't leak details everywhere doesn't immediately spring to mind.
radio_button* candidateRadioButton =
dynamic_cast<radio_button*>(candidate);
if (candidateRadioButton != 0)
candidateRadioButton->set_on_state(false);
}
candidate = &candidate->link_after();
}
}
iOnState = aOnState;
update();
if (is_on())
on.trigger();
else if (is_off())
off.trigger();
}
}

/Flibble

Öö Tiib

unread,

Feb 4, 2016, 7:31:29 AM2/4/16

to

May be have 'radio_button* radio_button::next_sibling()' instead of what seem
to be 'i_widget* radio_button::link_after()' and 'bool radio_button::is_sibling(i_widget*)'.

Mr Flibble

unread,

Feb 4, 2016, 7:46:02 AM2/4/16

to

link_after and is_sibling are members of widget base class but yes
"next_radio_button" would be better.

/Flibble

Mr Flibble

unread,

Feb 4, 2016, 7:56:21 AM2/4/16

to

const radio_button* radio_button::next_radio_button() const
{
const i_widget* candidate = &link_after();

while (candidate != this)
{
if (is_sibling(*candidate))
{
// Teh ghastly dynamic_cast! A simpler CLEAN solution which doesn't
leak details everywhere doesn't immediately spring to mind.

const radio_button* candidateRadioButton = dynamic_cast<const

radio_button*>(candidate);
if (candidateRadioButton != 0)

return candidateRadioButton;
}
candidate = &candidate->link_after();
}
return this;
}

radio_button* radio_button::next_radio_button()
{
return const_cast<radio_button*>(const_cast<const
radio_button*>(this)->next_radio_button());

}

void radio_button::set_on_state(bool aOnState)
{
if (iOnState != aOnState)
{
if (aOnState)

for (radio_button* nextRadioButton = next_radio_button();
nextRadioButton != this; nextRadioButton =
nextRadioButton->next_radio_button())
nextRadioButton->set_on_state(false);

Öö Tiib

unread,

Feb 4, 2016, 9:56:50 AM2/4/16

to

Hmm. Sometimes visitor pattern is used to search from or manipulate
elements of run-time hierarchy of objects of several types. It replaces
'dynamic_cast' with 2 virtual calls ('accept' of object calls 'visit' of visitor)
so it is not more efficient. Also some people hate the pattern. However
there won't be need for 'typeid' or 'dynamic_cast' with it.

Mr Flibble

unread,

Feb 4, 2016, 10:13:19 AM2/4/16

to

Visitor pattern wouldn't be appropriate in this instance as it would
involve creating a closed (class) leaky abstraction (the visitor interface).

/Flibble

Öö Tiib

unread,

Feb 4, 2016, 12:17:55 PM2/4/16

to

Why leaky? You can make the off-turning visitor as inner as you only
want. Usage is only slightly more code than typing 'dynamic_cast' and
checking that result is not 'nullptr'. Of course if you don't want to visit
your widgets for some other purposes as well then it is waste.

// that is the only copy-paste bloat that visitor pattern adds to
// visitable stuff
virtual void radio_button::accept(widget_visitor &v) override
{
v.visit(*this);

}

void radio_button::set_on_state(bool aOnState)
{
if (iOnState != aOnState)
{
if (aOnState)
{

struct off_others
:public widget_visitor // its 'visit' overloads do nothing
{
virtual void visit(radio_button& aThat) override { aThat.set_on_state(false); }
} offOthers();

for (i_widget* p = &link_after(); p != this; p = &p->link_after())
if (is_sibling(*p))
p->accept(offOthers);

Öö Tiib

unread,

Feb 4, 2016, 12:27:51 PM2/4/16

to

On Thursday, 4 February 2016 19:17:55 UTC+2, Öö Tiib wrote:
>
> // that is the only copy-paste bloat that visitor pattern adds to
> // visitable stuff
> virtual void radio_button::accept(widget_visitor &v) override
> {
> v.visit(*this);
> }
>
> void radio_button::set_on_state(bool aOnState)
> {
> if (iOnState != aOnState)
> {
> if (aOnState)
> {
> struct off_others
> :public widget_visitor // its 'visit' overloads do nothing
> {
> virtual void visit(radio_button& aThat) override { aThat.set_on_state(false); }
> } offOthers();

Typo ... 'offOthers;' without parentheses here otherwise it is vexing parse.

Mr Flibble

unread,

Feb 4, 2016, 1:52:19 PM2/4/16

to

It leaks into the widget_visitor interface which has to know about radio
buttons: widget_visitor would be a closed class yet would have to have a
visit method for each widget type which just isn't feasible.

/Flibble

Öö Tiib

unread,

Feb 4, 2016, 3:05:45 PM2/4/16

to

Ok, so then you are out of tools I suspect. Double dispatch is inappropriate.
Making 'set_on_state' virtual feels even worse. Things like boost::variant
and polymorphic_get can only result with 'dynamic_cast' written more
verbosely. So there is only 'dynamic_cast' left I suspect.