Sign in to follow this  
force_of_will

[C++] Is this the best way to solve my problem

Recommended Posts

I have the following class's
class ISound
{
public:

	ISound(void)
	{
	}

	virtual ~ISound(void)
	{
	}

protected:
	virtual bool Play() = 0;
	virtual bool Pause() = 0;
};


The reason i have play and pause protected is to prevent the user from using the functions directly, they will be handled by the sound system Now the next class
class ISoundSystem		
{
public:


	ISoundSystem(void){	}

	virtual ISoundHandler * CreateSound(const char *name_or_data, SOUND_MODE mode) = 0;
	virtual bool PlaySound(ISoundHandler *sound, ISound ** channel, bool loop) = 0;	
	virtual bool UnloadSound(ISoundHandler **sound) = 0;
	
	virtual bool PauseSound(ISound *channel) = 0;
	virtual bool Continue(ISound *channel) = 0;
	
	
	virtual void Update() = 0;

	virtual bool SetMasterVolume( float volume ) = 0; // set the master volume for all sounds (0.0 silent - 1.0 full volume )
	virtual bool IsPlaying() = 0;	// is any sound playing ?
	virtual ~ISoundSystem(void)	{ }
};


class SoundSystemFmod :	public ISoundSystem
{
public:

	SoundSystemFmod(void);
	
	ISoundHandler * CreateSound(const char *name_or_data, SOUND_MODE mode);

	// plays a sound previously loaded, the channel pointer can be NULL meaning that the user 
	// doesn't care to affect that particular instance of sound while it is playing
	// the user doesn't need to free the channel, the system takes care of all memory related to the channel
	bool PlaySound(ISoundHandler *handle, ISound ** channel,  bool loop);
	
	bool UnloadSound(ISoundHandler **handle);

	bool PauseSound(ISound *channel); 
	void Update();// should be called only once each frame (as noted in the FMOD api)
	bool Continue(ISound *channel);

	bool IsPlaying();
	bool SetMasterVolume(float volume);		 

	virtual ~SoundSystemFmod(void);


	static std::map<FMOD::Channel *,SoundFmod **> m_activeChannels;
//private:


	// for auto clean up (contains channels playing that get automatically cleaned by using a fmod callback)
		

	FMOD::System *m_system;
	FMOD::ChannelGroup *m_channelGroup;
	bool m_paused;
};


My problem is the following in class SoundSystemFmod, the function PlaySound will receive a base pointer to the class ISound and i want to call the Play method from ISound which is protected , in SoundFmod i have implemented the abstract functions as requested by the interface of ISound, however i need to access the Play method of ISound in SoundSystemFmod which is protected. I know i can declare in ISound, SoundSystemFmod as a friend, but that doesn't seems to be a good design, neither suits me, each time i would have implemented a class that inherits from ISoundSystem i would have to go to ISound and declare it as a friend. I tought also of in SoundFmod placing the protected methods i need as public. That way the user would work with the base class ISound and in the SoundSystemFmod i would do a cast like this example bool SoundSystemFmod::PauseSound(ISound *channel) { if (!channel) return false; return static_cast<SoundFmod *>(channel)->Play(); } this works but im kinda reluctant to the way i particularly solved the problem. Basicly i have this problem because friendship isn't inherited, otherwise a simple friend class ISoundSystem in ISound class, would make all class inherited from ISoundSystem be able to access the protected members from ISound Comments are very appreciated.

Share this post


Link to post
Share on other sites
I've realised that i could also simply add SoundSystemFmod as a friend in SoundFmod, and that way in the SoundSystemFmod::PlaySound method would just do


static_cast<SoundFmod*>(channel)->Pause();


which turns out to be a little better, since SoundFmod is related to SoundSystemFmod.

However i still wonder and bet there is a better way to sort out my problem.

Share this post


Link to post
Share on other sites
One way: make Play() and Pause() public. Is it really so bad?

Another way:


class ISoundSystem
{
public:
class ISound
{
// as you have it now
// ...
}

// and the rest
//...
};


By the way, I think this is a situation where judicious use of the template method pattern/NVI idiom would go a long way.

Share this post


Link to post
Share on other sites
Quote:
Original post by the_edd
One way: make Play() and Pause() public. Is it really so bad?


Yes it is :P, my sound sytem takes care of all the memory, and making Play and Pause public is just asking for him to do something like


ISoundHandler * handle = pSoundSystem->CreateSound(argv[1], SOUND_DEFAULT_MODE);
ISound *pSound;
pSoundSystem->PlaySound(handle, &pSound, false);

....
pSound->Play();


The problem is that pSound memory could already be free and the pointer set to null. Making it protected ensures the user can't access no method by using a ISound pointer. It however doesn't prevent him from deleting the pointer which won't have any harmfully effect since the pointer will be set to NULL.

I've fixed the problem by making ISoundSystem::Play / Pause methods non pure virtual. And added the simple code of

virtual bool Play(ISound *channel)
{
if(!channel)
return false;

return channel->Play();
}

ForceWill aka complicating easy problems...




Share this post


Link to post
Share on other sites
Quote:
Original post by force_of_will
Quote:
Original post by the_edd
One way: make Play() and Pause() public. Is it really so bad?


Yes it is :P, my sound sytem takes care of all the memory, and making Play and Pause public is just asking for him to do something like


ISoundHandler * handle = pSoundSystem->CreateSound(argv[1], SOUND_DEFAULT_MODE);
ISound *pSound;
pSoundSystem->PlaySound(handle, &pSound, false);

....
pSound->Play();


The problem is that pSound memory could already be free and the pointer set to null. Making it protected ensures the user can't access no method by using a ISound pointer.


You haven't discovered smart pointers yet, then? A good example.

Share this post


Link to post
Share on other sites
Quote:
Original post by the_edd
Quote:
Original post by force_of_will
Quote:
Original post by the_edd
One way: make Play() and Pause() public. Is it really so bad?


Yes it is :P, my sound sytem takes care of all the memory, and making Play and Pause public is just asking for him to do something like


ISoundHandler * handle = pSoundSystem->CreateSound(argv[1], SOUND_DEFAULT_MODE);
ISound *pSound;
pSoundSystem->PlaySound(handle, &pSound, false);

....
pSound->Play();


The problem is that pSound memory could already be free and the pointer set to null. Making it protected ensures the user can't access no method by using a ISound pointer.


You haven't discovered smart pointers yet, then? A good example.


And how smart pointer would help my case ?
I want the memory to be free when the sound stops playing (by means of a callback in fmod) , and not by reference count, but the user may still have the pointer, but the memory will be free and the pointer would be to null.

To my understanding smart pointer wouldn't help me :P


Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this