CODEWITHSUNDEEP
c++classoopnestedshared-ptr
James
James

Reputation: 359

How can I avoid this ugly nested class c++ code

I write games in C++ with SDL. I wrote games in C for more than a year and now I've been writing in C++ for the last 7 months. I'm trying to avoid almost all global variables and move to a system of ownership, where every object is owned by something else. This way I can just manage the life of classes who have shared_ptr members and almost never worry about freeing pointers.

For example, my game is a class with its subsystems.

class Game
{
public:
   Game();
   ~Game();
   void runFrame();

   std::shared_ptr<Display> display;
   std::shared_ptr<Audio> audio;
   std::shared_ptr<Save> save;
};

But I run into messy looking nested classes, like my audio class below.

class Audio
{
public:
   Audio(Game& parent);
   ~Audio();

   struct MusicFiles;
   struct SfxFiles;
   std::shared_ptr<MusicFiles> musicFiles;
   std::shared_ptr<SfxFiles> sfxFiles;

private:
   class Music
   {
   public:
      class File
      {
      public:
         File(Music& parent, std::string fileAddr);
         ~File();
         void play();
         void fadeIn();
         void stop();
      private:
         Music& _parent;
         std::string addr;
         Mix_Music* chunk;
      };

      Music(Audio& parent);
      ~Music();
      void setVolume();

   private:
      Audio& _parent;
      bool _enabled;
      int _volume;
   };

   class Sfx
   {
   public:
      class File
      {
      public:
         File(Sfx& parent, std::string fileAddr);
         ~File();
         void play();
         void stop();
      private:
         Sfx& _parent;
         std::string addr;
         Mix_Chunk* chunk;
         int channel;
      };

      Sfx(Audio& parent);
      ~Sfx();
      void setVolume();

   private:
      Audio& _parent;
      bool _enabled;
      int _volume;
   };

   Game& _parent;
   Music _music;
   Sfx _sfx;
};

I have nested classes because I dislike having to write "Music" or "Sfx" in every function name, like setMusicVolume(), setSfxVolume(), setMusicHook(), setSfxHook(), etc. etc. I can pull the nested classes out but Music and Sfx only need to exist within the Audio class. I'd rather reorganize everything and have a better design.

Do you have any better design suggestions?

Upvotes: 1

Views: 1409

Answers (2)

Daerst
Daerst

Reputation: 973

Organize your various classes into a namespace to group them, instead of a single class. This thing will become very hard to maintain once it grows. You want to use your classes to encapsulate functionality and provide a simple and comprehensive interface to the 'outside world'. Typically, each class will get its own header file (.h or .hpp) and compilation unit (.cpp).

Accept that there are many good reasons for using getter and setter functions - they are fundamental for good encapsulation. If you really have to, you can always use friend class Audio in your nested classes to make private and protected members visible to Audio, but not to all other classes in your project that may (at some time in the future, maybe) include these classes.

You say that you dislike writing audio.setMusicVolume(0.8f) instead of audio.music.setMusicVolume(0.8f). Some may say that audio.getMusic().setVolume(0.8f) would be a good compromise, but Demeter disagrees. By using wrapper functions like setMusicVolume, every other class only needs to know about and communicate with Audio while Music is strictly internal and only known by Audio itself. If you use function chaining, you lose this advantage and expose Music and Sfx to the world - which is not necessarily a bad thing. If you want to stick to your syntax, my suggestion would be to keep the public interfaces very small and use friend to expose more functionality to Audio, if you need to.

Upvotes: 4

Ben Voigt
Ben Voigt

Reputation: 283733

FYI, in addition to using namespaces, nested types can be placed out of line, just like member functions, by using their qualified name. So you can do:

class Audio
{
public:
   Audio(Game& parent);
   ~Audio();

   struct MusicFiles;
   struct SfxFiles;
   std::shared_ptr<MusicFiles> musicFiles;
   std::shared_ptr<SfxFiles> sfxFiles;

private:
   class Music;
};

in another file

#include "Audio.h"

class Audio::Music
{
public:
  class File;

  Music(Audio& parent);
  ~Music();
  void setVolume();

private:
  Audio& _parent;
  bool _enabled;
  int _volume;
};

Upvotes: 2

Related Questions