How to avoid many static casts and nullptr checks in c++?

Question

So a large legacy code base has structure that looks something like this.

We have a base app that has this public interface:

// Base app
class BaseApp {
public:
   ...
   Widget* getUiWidget(string _widgetName);
   ...
}

However, our widgets are implemented like this

// Widget structure
class Widget {
...
}

class Button : public Widget {
...
}

class Label : public Widget {
...
}

The problem is everywhere in our view code we have a bunch of calls that look like this:

auto button = static_cast<Button*>(getUiWidget(buttonName));
if (button != nullptr) {
  // something happens
  button->doSomething1();
}

auto label = static_cast<Label*>(getUiWidget(buttonName));
if (label != nullptr) {
  // something happens
  label->doSomething2();
}

I personally don't like this as there are literally thousands of calls like this and I feel it could be done much better.

My intuition tells me I can do something like this

// Base app
class BaseApp {
public:
   ...
   Widget* getUiWidget(string _widgetName);
   Label* getLabel(string _labelName) {
     if (//labelName is in widget layout) 
        return new NullLabel;
     else 
        return new Label(_labelName)
   }
   Button* getButton(string _buttonName) {
     if (//buttonName is in widget layout) 
        return new NullButton;
     else 
        return new Button(_buttonName)
   }
   ...
}

Where the null objects are

class NullButton : public Button {
public:
  // override all methods of button and do nothing with it
}

class NullLabel : public Label {
public:
  // override all methods of Label and do nothing with it
}

So the code :

auto button = static_cast<Button*>(getUiWidget(buttonName));
if (button != nullptr) {
  // something happens
  button->doSomething();
}

// Would turn in to 

auto button = getButton(buttonName);
button->doSomething1();

My issue with this approach is that we have n number of widget children like toggle, slider, etc. So, in order to implement this, I would need n number of getSomeWidgetChild and also I would need to write n number of NullSomeWidgetChild.

Is there a better solution?

[Are there any valid use cases to use new and delete, raw pointers or c-style arrays with modern C++?](https://stackoverflow.com/questions/46991224/are-there-any-valid-use-cases-to-use-new-and-delete-raw-pointers-or-c-style-arr) — user0042, Dec 13 '17 at 19:00
Use an interface based design and references or smart pointers. Wrap the whole legacy stuff away. — user0042, Dec 13 '17 at 19:02
Why do you need the NullLabel type? Why not just return nullptr? I can see an argument for the getLabel call over littering the code with casts but returning an object instance when there is no item with the given name seems very odd. — SoronelHaetir, Dec 13 '17 at 19:09
Should you be using `dynamic_cast<>` instead of `static_cast<>` to catch issues with casting to the wrong type? — pcarter, Dec 13 '17 at 19:11
Using `static_cast` with the wrong derived type is undefined behavior, and it certainly isn't obligated to return `nullptr`. You are thinking of `dynamic_cast`. — François Andrieux, Dec 13 '17 at 19:36
It sounds to me like you need a virtual function. Read about polymorphism. — François Andrieux, Dec 13 '17 at 19:37
You could make `getUiWidget()` be a template function so the caller can specify the output type, that will eliminate the need for the caller to use `static_cast`/`dynamic_cast` (if you make the function take the returned pointer as an output parameter, it can deduce the template automatically). Making the function throw an exception if the widget name is not found (or the widget is the wrong type) will eliminate the `nullptr` checks. — Remy Lebeau, Dec 13 '17 at 20:58

Shivendra Agarwal · Answer 1 · 2017-12-13T19:47:17.613

can use a visitor pattern to avoid dynamic casts. (static casts as marked in questions are hazardous!)

Define a new accept method in widget hierarchy.
Create a new visitor class encompassing all the object specific "do something" you wanted to do in the overloaded visit functions.

adding visitor class:

class Visitor {
public visit (Button* button) { /* do button specific work */}
public visit (Label* label) { /* do label specific work */}
}

adding accept method:

// Widget structure
class Widget {
virtual public accept (Visitor * v);
...
}

class Button : public Widget {
public accept (Visitor * v) override {v->visit(this);}
...
}

class Label : public Widget {
public accept (Visitor * v) override {v->visit(this);}
...
}

Now your code will change to:

Visitor* v = new Visitor;
auto button = getUiWidget(buttonName);
button.accept(v);

score 0 · Answer 2 · answered Dec 13 '17 at 19:14

Usually you make use of polymorphism and dynamic binding to overcome the need of even casting the returned object:

class Widget {
  virtual void someTypeSpeficifBehaviour() = 0;
}
class Button : public Widget {
  virtual void someTypeSpeficifBehaviour() { // button specific };
}
class Label : public Widget {
  virtual void someTypeSpeficifBehaviour() { // label specific };
}
int main() {
  Widget *w = baseApp->getUIWidget("test");
  w->someTypeSpeficiBehaviour();
}

It you cannot handle it that way, you can still include a type information in the object:

class Widget {
  virtual bool isButton() = 0;
}
class Button : public Widget {
  virtual bool isButton() { return true; }
}
class Label : public Widget {
  virtual bool isButton() { return false; }
}
int main() {
  Widget *w = baseApp->getUIWidget("test"); 
  if (w->isButton()) { // button specific
  }
  else { // else...
  }
}

If that all does not work, you may think of dynamic_cast<>-checks (not static_cast). But usually a dynamic_cast<> should make you rethinking the design.

I can't do the first because the method signatures are not always the same. And I can't do the second because that means every widget knows about all its brothers. For example, label should not need to know anything about button right? — user3904534, Dec 13 '17 at 19:21

score 0 · Answer 3 · edited Jun 20 '20 at 09:12

Your sample code is unsafe.

As static_cast does no run-time type information (RTTI) checks before casting, those nullptr checks will not catch casting failures.

Using RTTI one would expect those to be dynamic_cast.

Replacing test casts without invoking RTTI when handling polymorphism would require some form of enumeration of the widget types and storage of the type enumeration in the class as a static const.

If you don't mind RTTI then use typeid and switch on the result.

Since the type of a Button or Label and the associated hash_code are known constants at compile time a switch should work fine, but if I'm wrong you can always use if statements.

e.g.:

auto widget = getUiWidget(buttonName);
switch(typeid(*widget).hash_code())
{
    case typeid(Button).hash_code():
        {cast to Button, possibly do something, and break}
    case typeid(Label).hash_code():
        {cast to Label, possibly do something, and break}
}

How to avoid many static casts and nullptr checks in c++?

3 Answers3

Your sample code is unsafe.