Jump to content

  • Log In with Google      Sign In   
  • Create Account

Interface woes, windows header and coupling


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
3 replies to this topic

#1 KaiserJohan   Members   -  Reputation: 1156

Like
0Likes
Like

Posted 13 September 2012 - 03:08 AM

Hello,

Kinda vague title, but I'll try to explain below:

I have two interfaces, IMutex and IConditionVariable, and behind them the concrete implementations.
These are created/destroyed by a factory class.

IMutex looks like this:

class IMutex
		{
		public:
				enum MutexState
				{
						UNLOCKED = 0,
						LOCKED
				};
				virtual ~IMutex() { }
				virtual int32_t Lock() = 0;
				virtual int32_t Unlock() = 0;
				virtual const MutexState& GetMutexState() const = 0;
		};

IConditionVariable looks like this:

class IConditionVariable
		{
		public:
				enum ConditionVariableState
				{
						READY,
						WAITING
				};
				virtual ~IConditionVariable() { }
				virtual void Wait(IMutex* mutex) = 0;
				virtual void TimedWait(IMutex* mutex, uint32_t milliseconds) = 0;
				virtual void Signal() = 0;
				virtual void Broadcast() = 0;
				virtual const ConditionVariableState& GetConditionVariableState() const = 0;
		};

Two question:

1.
In the implementation of condition variable, I need to have the native mutex handle behind the IMutex so I can pass it to pthreads/windows api. But currently there is no way to aquire it.

To this I have two solutions:
  • Make IConditionVariable a friend of IMutex, typedef the native handle and make it a protected member variable. Don't like this one very much, they shouldn't be coupled?
  • Add a method to IMutex "GetNativeHandle" which returns the underlying platform handle. Also requries me to typedef the native handle. I like this one better, but I'd rather encapsulate the handle.
I prefer the second option, but is there any other way?


2,
In both cases above, I need to typedef the native handle in the interface, either to return it or to declare it as a protected member

#if defined _WIN32 || _WIN64
						typedef CRITICAL_SECTION MutexHandle;
				#else
						typedef pthread_mutex_t MutexHandle;
				#endif

But, for windows I need the definition of CRITICAL_SECTION. The only solution I have found is to include the <windows.h> with WIN32_LEAN_AND_MEAN... but I'd rather not have to include the windows header in an interface file.
How can I solve this one?

Edited by KaiserJohan, 13 September 2012 - 03:11 AM.


Sponsor:

#2 Hodgman   Moderators   -  Reputation: 30384

Like
0Likes
Like

Posted 13 September 2012 - 03:50 AM

IMHO, it's not a big deal if those two classes are coupled, because they're both such small thin wrappers around different parts of the same API. I'd probably even implement them in the same file.
I wouldn't want to expose the native handle type via a typedef, as this requires that the user of "IMutex" has to include the windows headers -- preferably, only the CPP that implements the mutex interface should have to carry that burden.
Lastly, I wouldn't bother using polymorphism here, as you're deciding which implementation of the interface to use at compile-time, not run-time.

If the internal types are being hidden behind a factory API, I'd use something like:
//threadtypes.h
class Mutex
{
public:
	int32_t Lock();
protected:
	Mutex(){}
};
class ConditionVariable
{
public:
	void Wait(Mutex* mutex);
protected:
	ConditionVariable(){}
};

//threadtypes.cpp
class MutexWin32 : public Mutex
{
public:
	CRITICAL_SECTION cs;
}
namespace {
	MutexWin32& Members(Mutex& p) { return *(MutexWin32*)&p; }
}

int32_t Mutex::Lock() { EnterCriticalSection(&Members(*this).cs); }

void ConditionVariable::Wait(Mutex& mutex)
{
	CRITICAL_SECTION& cs = Members(mutex).cs;
	...
}


#3 KaiserJohan   Members   -  Reputation: 1156

Like
0Likes
Like

Posted 15 September 2012 - 02:06 PM

How can you design this Without having a factory class creating it btw, and still avoiding having to include windows header?

Edited by KaiserJohan, 15 September 2012 - 02:16 PM.


#4 Hodgman   Moderators   -  Reputation: 30384

Like
0Likes
Like

Posted 16 September 2012 - 12:35 AM

Here's a few options:

1) Use factory functions
//threadtypes.h
class Mutex
{
public:
	static Mutex* Create();
	static void Release(Mutex*);
};

//threadtypes.cpp
Mutex* Mutex::Create() { return new MutexWin32; }
void Mutex::Release(Mutex* m) { delete (MutexWin32*)m; }
2) split it over two headers. Regular users of the mutex only need the regular "*.h", but creators of mutexes need the bloated "*_imp.h" header.
//threadtypes.h
class Mutex
{
        ...
protected:
        Mutex(){}
};

//threadtypes_imp.h
#include "threadtypes.h"
#include "windows.h"
class MutexWin32 : public Mutex
{
        ...
3) Use PIMPL
//threadtypes.h
class Mutex
{
public:
        Mutex();
        ~Mutex();
        int32_t Lock();

private://non-copyable:
	Mutex(const Mutex&);
	Mutex& operator=( const Mutex& );

	void* pimpl;
};

//threadtypes.cpp
class MutexWin32
{
public:
        int32_t Lock() {...}
private:
        CRITICAL_SECTION cs;
}

Mutex::Mutex() : pimpl(new MutexWin32) {}
Mutex::~Mutex()  { delete (MutexWin32*)pimpl; }
int32_t Mutex::Lock() { return (((MutexWin32*)pimpl)->Lock(); }





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS