How to avoid dynamic_cast/static_cast

Hi,

I've read more than once in this forum (and by people who I believe know what they are talking about), that if your code makes use of dynamic_cast/static_cast it should be redesigned.

Unfortunately my code is full of static_cast and I'd be very curious to understand how I can avoid them. To illustrate, below is some code which is similar to some of the data structures I have in my programs:

Code:

---

class FSObject
{
public:
  virtual bool isFile() const = 0;
  virtual bool isFolder() const = 0;
  // ...
}

class Folder : public FSObject
{
public:
  class iterator;
public:
  virtual bool isFile() const { return false; }
  virtual bool isFolder() const { return true; }
  iterator begin();
  iterator end();
  // ...
private:
  vector<FSObject*> containedItems;
}

class File : public FSObject
{
public:
  virtual bool isFile() const { return true; }
  virtual bool isFolder() const { return false; }
  unsigned long size() const;
  // ...
}

---

The FSObject class represents any file system objects. It is inherited by File and Folder which are specializations to file system objects. A folder, as we all know, can contain file system objects which can be files or folders. The items within a folder can be accessed via an iterator.

So far, so good, I would say it is sensible design. To keep my code as portable as possible, I am not relying on compiler-provided RTTI, but provide the functions ifFile() and isFolder().

Now an example of a function where I have to cast around:

Code:

---

// Search recursively for a file, for which a call to func returns true
File * recursive_search( Folder * fld, bool (*func)(const File&) )
{
  for ( Folder::iterator it = fld.begin(); it != fld.end(); ++it ) {
    if ( (*it)->isFolder() ) {
      File * result = recursive_search( static_cast<Folder*>(*it), f );
      if ( result ) return result;
    } else { // (*it)->isFile()
      File * file = static_cast<File*>(*it);
      if ( func(*file) ) return file;
    }
  }
  return 0;
}

---

A simple recursive search for a file. As stated above, I'm doing my own rtti using the isF.. functions, therefore I'm using static and not dynamic casts.

Now, what would be a sensible way to avoid the casts? Moving functionality like the iterating to the FSObject class cannot really be the answer.

I'll be happy to read any suggestions, on how to design this better. Note that the code provided here is meant to be illustrative, not complete, optimal or compilable.

What can be FSObject be except File or Folder? Why not to drop this design approach and stick more simple one:

Code:

---

class Record {
  enum Type {dir, file, link, ..., etc};
  Type type_;
  //holds subdirs and files if the current record is a directory oterwise it's empty
  vector<Record*> containedItems;
public:
  bool isFile() const;
  bool isDir() const;
  bool isLink() const;
//and so on
};

---

Quote:

---

Originally Posted by AvDav

What can be FSObject be except File or Folder? Why not to drop this design approach and stick more simple one

---

Well, why not?

• Bloated objects: Every object will contain much more attributes than it needs and take up more space in memory
• No static type safety: A function that would expect a File object could be passed a Folder object and vice-versa
• Not extensible: Adding an additional object type would require large code modifications

Implement AsFile() and AsFolder() virtual functions. Implement the proper one to return "this" and the improper one to throw an exception (in case comeone is stupid and doesnt check first). No casts.

Well, your function asks for a pointer to a Folder, but the iterator is setup to provide a reference to a Folder as the default alternate conversion.

Change your function to accept a Folder reference instead and the confusion presented to the compiler is alleviated.

However, I'm not sure that cast COULD compile, because it would cast a Folder to a Folder * - nonsensical UNLESS the Folder has a conversion function to a pointer of itself (which isn't likely).

In fact, the *it should return a Folder, so if the function accepted a Folder &, all would be well.

Despite warnings to the contrary, the casting operators are there for situations where they should be used. Overuse is usually a sign that design should be reconsidered, but that doesn't mean the casting operators should be avoided when situations require it.

Further, RTTI information is automatically part of the build if exceptions are turned on, and exceptions should be turned on. Portability across compilers may be your limiting factor, but maximum portability is not so often a demand (unless you're making cross platform development tools). Portability is more often about operating system and CPU independence than compiler portability, but that's your call, no doubt.

Use and overuse of dynamic casting usually means virtual functions were indicated, but avoided. Dynamic casting has one particular place where it has no replacement - casting in a multiple inheritance hierarchy.

While many shun multiple inheritance, it can be useful to adapt an existing framework you don't own with common behavior. There can be times when you have a common behavior to apply to disparate framework components, and a second parent does this quite well.

However, if containment by that new common base member is desired, the problem exists whereby the pointer must be adjusted. If, for example, you receive a new object that has multiple parents, but you want to store the object according to one of the secondary parents, not the base most parent, the pointer must be adjusted to reflect the position of that secondary parent. A dynamic cast is the only way to do that.

In early versions of the MS VC Compiler (4.2 I think, maybe it was 4.1, whichever was their first to allow MI), the dynamic cast operator couldn't do this math manipulation - it was broken.

That's no longer an issue, of course, but my point is the casting operators exist to be used (but like anything, not abused).

goto, on the other hand, is an assembler like vestigial wart that should have been surgically removed :)

Quote:

---

Originally Posted by treuss

Well, why not?

• Bloated objects: Every object will contain much more attributes than it needs and take up more space in memory
• No static type safety: A function that would expect a File object could be passed a Folder object and vice-versa
• Not extensible: Adding an additional object type would require large code modifications

---

1. Not much more, only 1 data member, 4-10 bytes :)
2. Thats why, isFile, isXXX() functions for to determine the type
3. Not really large, just item in Type enumeration and 1 isXXType() identifier function, thats all.

you use dynamic_cast (or static_cast) because you break typing somewhere by inserting a file or a folder in a general vector. you use static_cast (or dynamic_cast) to recover from the loss.

if you don't want to break strong typing, thing of your folders as a container of :
- files
-other folders

and treat them accordingly.

For instance :

Code:

---

class FSObject
{
  // ...
}

class Folder : public FSObject
{
public:
  class file_iterator;
  class folder_iterator;
public:
  files_iterator files_begin();
  files_iterator files_end();
  folder_iterator subfolders_begin();
  folder_iterator subfolders_end();
  // ...
private:
  vector<Files*> containedFiles;
  vector<Folder*> containedFolders;
}

class File : public FSObject
{
public:
  unsigned long size() const;
  // ...
}

---

and then, the calling code doesn't have to check the type, the folder class provides it

Code:

---

// Search recursively for a file, for which a call to func returns true
File * recursive_search( Folder * fld, bool (*func)(const File&) )
{
  for ( Folder::folders_iterator it = fld.folders_begin(); it != fld.folders_end(); ++it )
  {
      return recursive_search( static_cast<Folder*>(*it), f );
  }
  for ( Folder::files_iterator it = fld.files_begin(); it != fld.files_end(); ++it )
  {
      if ( func(**it) ) return file;
  }
  return 0;
}

---

don't lose typing if you want to avoid dynamic_cast or testing a virtual function. You once had enough information and dynamic_cast is a way to recoer the information that was _lost_.

I think this is better:

Code:

---

FSObject *recursive_search(FSObject *fld, bool (*func)(const FSObject&));

---

Now maybe the cast is not necessary.

yeah but as i read the code again i think it simply doesn't work.

If a file is a folder you go into the folder directly and retrn the value, so you will only scan the first folder that has no folder. The return statements will then completely overlook the files! the for loop is not executing since the first item will already call return.

Code:

---

// Search recursively for a file, for which a call to func returns true
File * recursive_search( Folder * fld, bool (*func)(const File&) )
{
  for ( Folder::files_iterator it = fld.files_begin(); it != fld.files_end(); ++it )
  {
      if ( func(**it) ) return file;
  }
  for ( Folder::folders_iterator it = fld.folders_begin(); it != fld.folders_end(); ++it )
  {
      File* result = recursive_search( *it, f );
      if(result) retur result;
  }
  return 0;
}

---

Code:

---

FSObject *recursive_search(FSObject *fld, bool (*func)(const FSObject&))
{
  for ( FSObject::iterator it = fld.begin(); it != fld.end(); ++it ) {
    if ( (*it)->isFolder() ) {
      return recursive_search( *it, f );
    } else { // (*it)->isFile()
      if ( func(*it) ) return it;
    }
  }
  return 0;
}

---

the begin()/end() is not part of a FSObject but of a Folder.

Quote:

---

Originally Posted by TheCPUWizard

Implement AsFile() and AsFolder() virtual functions. Implement the proper one to return "this" and the improper one to throw an exception (in case comeone is stupid and doesnt check first). No casts.

---

Nice one. Only backdraw I see, is that another virtual function call would probably cost some CPU time (unless the compiler optimizes very good), whereas the static_cast does not result in any code to be executed.

Quote:

---

Originally Posted by screetch

yeah but as i read the code again i think it simply doesn't work.

---

Well spotted. I corrected it to behave correctly.

Quote:

---

Originally Posted by screetch

if you don't want to break strong typing, thing of your folders as a container of :
- files
- other folders

---

I thought about that possibility, but this way I am losing the information in which order the files/folders are sorted. In my concrete program, that is relevant.

In other words, a recursive search using these two vectors would possibly return a different result than the recursive search I describe.

ok i didn't think the order was important. I overlook the client code and i thought that you added files that were found to a list so that order wasn't important.


you have to find a way that preserves both strong typing and the order regardless of the type if you want to avoid dynamic_casts.

you can store the index in a separate vector for example : an iterator tells you what's the type to take, you start with files_begin() and folder_begin() simultaneously. Another iterator is type_begin() (returning an enum which tells if the next item is a folder or a type)

Code:

---

// Search recursively for a file, for which a call to func returns true
File * recursive_search( Folder * fld, bool (*func)(const File&) )
{
  Folder::folders_iterator folders = fld.folders_begin();
  Folder::files_iterator files = fld.files_begin();

  for ( Folder::type_iterator it = fld.types_begin(); it != fld.types_end(); ++it )
  {
      switch (*it)
      {
        case FILE:
            assert(files_iterator <> fld.files_end());
            if(func(**files_iterator)) return *files_iterator;
            ++files_iterator;
            break;
        case FOLDER:
            assert(folders_iterator <> fld.folders_end());
            {
              File* result = recursive_search(**folders_iterator);
              if(result) return result;
            }
            ++folders_iterator;
            break;
        default:assert(false); break;
      }
  }
  // here, if you reached the end of types, you must have reached the end of files and folders
  assert(files_iterator == fld.files_end());
  assert(folders_iterator == fld.folders_end());
  return 0;
}

---

it seems complicated but i'm sure several other operations that work only on files or on folders will be easier to write. When the order matters it's a bit difficult but when order doesn't matter (i think it's often the case) you haven't lost strong typing.