1

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?

James
  • 359
  • 2
  • 12
  • If you dislike having functions like setters and getters, then don't use C++ or any OOP language. – Silex Jan 27 '15 at 17:42
  • You don't need to use **nested** classes. You can put the classes in seperate files and still include the header files. – Thomas Matthews Jan 27 '15 at 17:45
  • @Silex: To clarify, I love setters and getters, I dislike having to include the word Music or Sfx in every function name. Instead I like audio.music.setVolume – James Jan 27 '15 at 17:46
  • @Silex: Or use C++ without too much OOP. – Mike Seymour Jan 27 '15 at 17:46
  • @MikeSeymour: I want to use more OOP. I'm just curious as to better ways to organize it all – James Jan 27 '15 at 17:47
  • You can make the class definition less messy, without changing the logical structure, by keeping the classes nested, but moving their definitions outside the outer class (just declaring them there). – Mike Seymour Jan 27 '15 at 17:50
  • Why do you have to include Music or Sfx in every function name? Just have a separate class for Music for example and have a private object of that class inside Audio. Then write a getter for that private object and call a setter on that getter => audio->getMusic()->setVolume(...) for example. – Silex Jan 27 '15 at 18:05
  • @Silex: That is what I'm doing, essentially, audio.music().setVolume(). I meant I could avoid having a Music or Sfx class altogether by using audio.setMusicVolume(), but I dislike specifying "Music" in every function name. – James Jan 27 '15 at 18:19
  • I think Ben Voigt solution fits your needs the best then! – Silex Jan 27 '15 at 18:34
  • Also looks like there's opportunity for some inheritance, which might simplify things, as there is commonality between a `Sfx` and a `Music` (and their scoped `File` classes) – Rowland Shaw Jan 28 '15 at 08:43

2 Answers2

4

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.

Community
  • 1
  • 1
Daerst
  • 954
  • 7
  • 24
  • To repeat my comment, I love setters and getters, I dislike having to include the word Music or Sfx in every function name. Instead I like audio.music.setVolume – James Jan 27 '15 at 18:01
  • I extended my answer to take your preference for the specific syntax into account. – Daerst Jan 28 '15 at 11:52
2

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;
};
Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • Good solution if you wanna hold on to nested classes. – Silex Jan 27 '15 at 18:08
  • I have no attachment to nested classes. I just used them because Music and Sfx have no implementation elsewhere. I don't mind changing everything if someone has a better idea. – James Jan 27 '15 at 18:13
  • Thank you! For the majority of my decluttering I placed my nested classes out of line. Thank you to Daerst's answer also. In several places I have refrained from designs like audio->music()->getVolume(), and instead simplified using audio->getMusicVolume() – James Feb 01 '15 at 01:26