Trick to prevent conversion

[Tomás Ó hÉilidhe]

(I'm simplifying this problem greaty in the following code but you get
the idea)

I have a class something like:

class Processor {
public:
unsigned GetCurrentProcess(void);
unsigned GetCurrentThread(void);

void DoSomethingWithProcess(unsigned process_id);
void DoSomethingWithThread(unsigned thread_id);
};

The function, GetCurrentProcess, returns an unsigned integer value
which represents a process.
The function, GetCurrentThread, returns an unsigned integer value
which represents a thread.

The problem is, that it's very easy to mistakenly take the value from
GetCurrentProcess and pass it as an argument to DoSomethingWithThread.
(I've done it already!). Therefore, I want two different types for these
identifiers, two types that can't convert to each other. At the moment I
have the following:

class Processor {
public:

struct ProcessHandle { unsigned i; };
struct ThreadHandle { unsigned i; };

ProcessHandle GetCurrentProcess(void);
ThreadHandle GetCurrentThread(void);

void DoSomethingWithProcess(ProcessHandle process_id);
void DoSomethingWithThread(ThreadHandle thread_id);
};

Of course, this does the trick. But I'm wondering if there's a more
elegant way of doing it? What other methods are there for making types
incompatible with each other?

(My actual code is far more complicated than this -- obviously I wouldn't
be very competant if I made such a stupid mistake in code as simple as
this).

 

[Victor Bazarov]

(I'm simplifying this problem greaty in the following code but you get
the idea)

I have a class something like:

class Processor {
public:
unsigned GetCurrentProcess(void);
unsigned GetCurrentThread(void);

If you can, perhaps you would get in the habit of omitting 'void'
for a function with no arguments. I know I'd appreciate it because
your code would be easier to read and understand.

Also, it seems that those functions should perhaps be 'const'. If
they don't change anything in 'Processor', of course.

void DoSomethingWithProcess(unsigned process_id);
void DoSomethingWithThread(unsigned thread_id);
};

The function, GetCurrentProcess, returns an unsigned integer value
which represents a process.
The function, GetCurrentThread, returns an unsigned integer value
which represents a thread.

The problem is, that it's very easy to mistakenly take the value from
GetCurrentProcess and pass it as an argument to DoSomethingWithThread.
(I've done it already!). Therefore, I want two different types for
these identifiers, two types that can't convert to each other. At the
moment I have the following:

class Processor {
public:

struct ProcessHandle { unsigned i; };
struct ThreadHandle { unsigned i; };

ProcessHandle GetCurrentProcess(void);
ThreadHandle GetCurrentThread(void);

void DoSomethingWithProcess(ProcessHandle process_id);
void DoSomethingWithThread(ThreadHandle thread_id);
};

Of course, this does the trick. But I'm wondering if there's a more
elegant way of doing it? What other methods are there for making types
incompatible with each other?
Enums.

(My actual code is far more complicated than this -- obviously I
wouldn't be very competant if I made such a stupid mistake in code as
simple as this).

V

 

[Tomás Ó hÉilidhe]

If you can, perhaps you would get in the habit of omitting 'void'
for a function with no arguments. I know I'd appreciate it because
your code would be easier to read and understand.

I hadn't even realised I'd written "void"; I program in C and it just
feels more natural to me, more verbose.

Also, it seems that those functions should perhaps be 'const'. If
they don't change anything in 'Processor', of course.

Ah yes, of course, I'm a little rusty.



I have a list box on a dialog box. The list box contains items, and each
item has an index. First item = 0, second item = 1, third item = 3, you
get the idea.

I have a function that returns the text of one of those items:

char const *GetItemText(unsigned const i);

"unsigned" is definitely a natural choice for this, because the index is
nothing more than an integer. However, I've been using:

struct ItemHandle { unsigned i; };

so that the programmer definitely won't supply the function with a
different kind of index. I've at least three kinds of closely-related-
but-different indices floating around in the same function, and so I
need some extra reassurance that the programmer won't mix them up. Hence
the want for types that won't convert to each other.

I've already got a solution that works; maybe I should just leave it at
that.

 

[tragomaskhalos]

The problem is, that it's very easy to mistakenly take the value from
GetCurrentProcess and pass it as an argument to DoSomethingWithThread.
(I've done it already!). Therefore, I want two different types for these
identifiers, two types that can't convert to each other. At the moment I
have the following:

class Processor {
public:

    struct ProcessHandle { unsigned i; };
    struct ThreadHandle { unsigned i; };

    ProcessHandle GetCurrentProcess(void);
    ThreadHandle GetCurrentThread(void);

    void DoSomethingWithProcess(ProcessHandle process_id);
    void DoSomethingWithThread(ThreadHandle thread_id);

};

Of course, this does the trick. But I'm wondering if there's a more
elegant way of doing it? What other methods are there for making types
incompatible with each other?

I think this is a pretty good way of doing it. One way of making it
a bit more elegant/generic would be something like this
(warning - untested):

template <typename W, class TAG> class TypeWrapper {
W myWrapped;
public:
explicit TypeWrapper(W wrapped) : myWrapped(wrapped) {}
W getWrapped() { return myWrapped; }
};

#define TYPE_WRAPPER(W, X) \
struct X##Tag{}; \
typedef TypeWrapper<W, X##Tag> X

This could be in a generic header file, then you could say:
// define types ...
TYPE_WRAPPER(unsigned, ProcessHandle);
TYPE_WRAPPER(unsigned, ThreadHandle);

// use them
ProcessHandle ph(some_unsigned_var);
unsigned x = ph.getWrapped();
// etc but
ThreadHandle th = ph; // fails to compile

 

[Victor Bazarov]

[..]
I have a list box on a dialog box. The list box contains items, and
each item has an index. First item = 0, second item = 1, third item =
3, you get the idea.

I have a function that returns the text of one of those items:

char const *GetItemText(unsigned const i);

"unsigned" is definitely a natural choice for this, because the index
is nothing more than an integer. However, I've been using:

struct ItemHandle { unsigned i; };

so that the programmer definitely won't supply the function with a
different kind of index. I've at least three kinds of closely-related-
but-different indices floating around in the same function, and so I
need some extra reassurance that the programmer won't mix them up.
Hence the want for types that won't convert to each other.

I've already got a solution that works; maybe I should just leave it
at that.

The point is that it's not obscure to the point that somebody reading
the code won't understand what's it for.

As 'tragomaskhalos' suggested, you might want to give those structs
a proper constructor, then you can still make an ItemHandle out of
an unsigned variable without creating a local object, and also you
will be less prone to use a "handle" without initialising it to some
specific value.

V

 

[Tomás Ó hÉilidhe]

I think this is a pretty good way of doing it. One way of making it
a bit more elegant/generic would be something like this
(warning - untested):

template <typename W, class TAG> class TypeWrapper {
W myWrapped;
public:
explicit TypeWrapper(W wrapped) : myWrapped(wrapped) {}
W getWrapped() { return myWrapped; }
};

#define TYPE_WRAPPER(W, X) \
struct X##Tag{}; \
typedef TypeWrapper<W, X##Tag> X

This could be in a generic header file, then you could say:
// define types ...
TYPE_WRAPPER(unsigned, ProcessHandle);
TYPE_WRAPPER(unsigned, ThreadHandle);

// use them
ProcessHandle ph(some_unsigned_var);
unsigned x = ph.getWrapped();
// etc but
ThreadHandle th = ph;

Honestly I think that's a bit much. Firstly, I'd want the type to remain
a POD, so constructors and private data are out the window. Also I
wouldn't want member functions coming into it.

I think I was right on track with my struct solution.

 

[Tomás Ó hÉilidhe]

The point is that it's not obscure to the point that somebody reading
the code won't understand what's it for.

As 'tragomaskhalos' suggested, you might want to give those structs
a proper constructor, then you can still make an ItemHandle out of
an unsigned variable without creating a local object, and also you
will be less prone to use a "handle" without initialising it to some
specific value.

No, that's exactly what I *don't* want to do. I want ItemHandle to be
distinct from unsigned. I don't want their to be a conversion between the
two, neither implicit nor explicit.

The programmer using the reusable code doesn't need to know that ItemHandle
is a struct containing an unsigned int. All they need to do is store
ItemHandle's and pass them to other functions. For instance:

ItemHandle ih = GetItem(5);

DoSomethingWithItem(ih); /* OK */

DoSomethingWithItem(5); /* ERROR: Type mismatch */

 

[Victor Bazarov]

No, that's exactly what I *don't* want to do. I want ItemHandle to be
distinct from unsigned. I don't want their to be a conversion between
the two, neither implicit nor explicit.

The programmer using the reusable code doesn't need to know that
ItemHandle is a struct containing an unsigned int. All they need to
do is store ItemHandle's and pass them to other functions. For
instance:

ItemHandle ih = GetItem(5);

DoSomethingWithItem(ih); /* OK */

DoSomethingWithItem(5); /* ERROR: Type mismatch */

OK. Fine. You talk about absence of conversion, yet you yourself
introduce the conversion mechanism. It's called 'GetItem'. Instead
of writing

DoSomethingWithItem(5);

I _would_ (if I were to use your library) write

DoSomethingWithItem(GetItem(5));

Just to avoid using a local object which I don't need for anything
else. So, what's the point?

V

 

[Tomás Ó hÉilidhe]

@news.datemas.de:

OK. Fine. You talk about absence of conversion, yet you yourself
introduce the conversion mechanism. It's called 'GetItem'. Instead
of writing

DoSomethingWithItem(5);

I _would_ (if I were to use your library) write

DoSomethingWithItem(GetItem(5));

Just to avoid using a local object which I don't need for anything
else. So, what's the point?

It's a little hard to explain this without going into a long-winded
explanation of how the code works, but I'll try give a simplified
version.

* Let's say we have 4 items.
* Let's say we have 3 combinations, where each combination consists of a
series of items.
* The first combination consists of items 0 and 1.
* The second comination consists of items 0 and 2.
* The third cominations consists of item 0, 1 and 2.

When you query which items are contained in the third combination, you
get back a pointer to the first element of an array; this array contains
the indices of the items, i.e. {0,1,2}.

The indices of the combinations are very distinct from the indices of
the items but it's quite easy to mix them up by accident.

(Sorry I know that that's a poor explanation, but it would probably take
me an hour to explain the code if we were sitting across a table from
each other... let alone over the written word)

Suffice to say though, I want the index of a combination to be totally
incompatible with the index for a item, such that they can never be
mixed up. So instead of having:

unsigned combo, item;

, I have:

struct HandleCombo { unsigned i; } combo;
struct HandleItem { unsigned i; } item;

Now they can never be mixed up.

That's my solution to the problem, and it works. I only posted here
because I figured people may have encountered this problem before and
perhaps come up with a more elegant solution than I did.

It appears though that the struct way is the way to go.

 

[Jack Klein]

(I'm simplifying this problem greaty in the following code but you get
the idea)

I have a class something like:

class Processor {
public:
unsigned GetCurrentProcess(void);
unsigned GetCurrentThread(void);

void DoSomethingWithProcess(unsigned process_id);
void DoSomethingWithThread(unsigned thread_id);
};

The function, GetCurrentProcess, returns an unsigned integer value
which represents a process.
The function, GetCurrentThread, returns an unsigned integer value
which represents a thread.

The problem is, that it's very easy to mistakenly take the value from
GetCurrentProcess and pass it as an argument to DoSomethingWithThread.
(I've done it already!). Therefore, I want two different types for these
identifiers, two types that can't convert to each other. At the moment I
have the following:

class Processor {
public:

struct ProcessHandle { unsigned i; };
struct ThreadHandle { unsigned i; };

ProcessHandle GetCurrentProcess(void);
ThreadHandle GetCurrentThread(void);

void DoSomethingWithProcess(ProcessHandle process_id);
void DoSomethingWithThread(ThreadHandle thread_id);
};

Of course, this does the trick. But I'm wondering if there's a more
elegant way of doing it? What other methods are there for making types
incompatible with each other?

(My actual code is far more complicated than this -- obviously I wouldn't
be very competant if I made such a stupid mistake in code as simple as
this).

Why not two enumeration types, suitably defined so that hold the value
range you want, i.e.:

enum process_id { process_id_min = 0, process_id_max = UINT_MAX };
enum thread_id { thread_id_min = 0, process_id_max = UINT_MAX };

Modify your functions to accept and receive the enumerated types, with
suitable static_casts to and from unsigned int, as needed, inside the
functions, invisible to the caller. Modify your callers who get the
values to accept the appropriate enumeration type.

A C++ compiler will reject code where a caller retrieves a process_id
from one function and attempts to pass it to another that needs a
thread_id.

I even use this technique in C, where any decent compiler will warn
about mixing different enumeration types even though the language
permits it.

--
Jack Klein
Home: http://JK-Technology.Com
FAQs for
comp.lang.c http://c-faq.com/
comp.lang.c++ http://www.parashift.com/c++-faq-lite/
alt.comp.lang.learn.c-c++
http://www.club.cc.cmu.edu/~ajo/docs/FAQ-acllc.html

 

[vijayan]

> I think I was right on track with my struct solution.
yes, i think so too.

> Honestly I think that's a bit much. Firstly, I'd want the type to remain
> a POD, so constructors and private data are out the window. Also I
> wouldn't want member functions coming into it.
i think the only advantage of the handle being a POD could be from a performance perspective.
even if we have private data and member functions , the operations are trivial
and would be inlined by the compiler. so we could get type safety without any run time cost.
for example,

struct process_handle
{
  operator unsigned int() const { return h ; }
  private :
    unsigned int h ;
    explicit inline process_handle( unsigned int i ) : h(i) {}
    friend class processor ;
};

struct thread_handle
{
  operator unsigned int() const { return h ; }
  private :
    unsigned int h ;
    explicit inline thread_handle( unsigned int i ) : h(i) {}
    friend class processor ;
};

struct processor
{
  process_handle current_process() const { return process_handle(1) ; }
  thread_handle current_thread() const { return thread_handle(2) ; }
  unsigned int current_process_pod() const { return 1 ; }
  unsigned int current_thread_pod() const { return 2 ; }
};

unsigned int foo_handles( process_handle ph, thread_handle th )
{ return ph + th ; }

unsigned int bar_handles( processor pr )
{ return pr.current_process() + pr.current_thread() ; }

unsigned int foo_pods( unsigned int ph, unsigned int th )
{ return ph + th ; }

unsigned int bar_pods( processor pr )
{ return pr.current_process_pod() + pr.current_thread_pod() ; }

if we look at the generated assembly code, we can see that there is no difference between
using the encapsulated structures and a raw unsigned ints

> g++ -Wall -std=c++98 -pedantic -Werror -c -O3 -fomit-frame-pointer -S handle.cc

	.file	"handle.cc"
	.text
	.align 2
	.p2align 2,,3
.globl _Z8foo_podsjj
	.type	_Z8foo_podsjj, @function
_Z8foo_podsjj:
.LFB16:
	movl	8(%esp), %eax
	addl	4(%esp), %eax
	ret
.LFE16:
	.size	_Z8foo_podsjj, .-_Z8foo_podsjj
.globl __gxx_personality_v0
	.align 2
	.p2align 2,,3
.globl _Z8bar_pods9processor
	.type	_Z8bar_pods9processor, @function
_Z8bar_pods9processor:
.LFB17:
	movl	$3, %eax
	ret
.LFE17:
	.size	_Z8bar_pods9processor, .-_Z8bar_pods9processor
	.align 2
	.p2align 2,,3
.globl _Z11foo_handles14process_handle13thread_handle
	.type	_Z11foo_handles14process_handle13thread_handle, @function
_Z11foo_handles14process_handle13thread_handle:
.LFB14:
	movl	8(%esp), %eax
	addl	4(%esp), %eax
	ret
.LFE14:
	.size	_Z11foo_handles14process_handle13thread_handle, .-_Z11foo_handles14process_handle13thread_handle
	.align 2
	.p2align 2,,3
.globl _Z11bar_handles9processor
	.type	_Z11bar_handles9processor, @function
_Z11bar_handles9processor:
.LFB15:
	movl	$3, %eax
	ret
.LFE15:
	.size	_Z11bar_handles9processor, .-_Z11bar_handles9processor
	.ident	"GCC: (GNU) 4.2.3 20071024 (prerelease)"

 

[Erik Wikström]

@news.datemas.de:




It's a little hard to explain this without going into a long-winded
explanation of how the code works, but I'll try give a simplified
version.

* Let's say we have 4 items.
* Let's say we have 3 combinations, where each combination consists of a
series of items.
* The first combination consists of items 0 and 1.
* The second comination consists of items 0 and 2.
* The third cominations consists of item 0, 1 and 2.

When you query which items are contained in the third combination, you
get back a pointer to the first element of an array; this array contains
the indices of the items, i.e. {0,1,2}.

The indices of the combinations are very distinct from the indices of
the items but it's quite easy to mix them up by accident.

(Sorry I know that that's a poor explanation, but it would probably take
me an hour to explain the code if we were sitting across a table from
each other... let alone over the written word)

Suffice to say though, I want the index of a combination to be totally
incompatible with the index for a item, such that they can never be
mixed up. So instead of having:

unsigned combo, item;

, I have:

struct HandleCombo { unsigned i; } combo;
struct HandleItem { unsigned i; } item;

Now they can never be mixed up.

That's my solution to the problem, and it works. I only posted here
because I figured people may have encountered this problem before and
perhaps come up with a more elegant solution than I did.

It appears though that the struct way is the way to go.

IF you do not want them to be mixed up you must use different types for
them, creating new types are done using class, struct, or enum. An
alternative is to use run-time checks, so instead of using indexes you
use handles (which could be unsigned ints) where each handles value is
some reversible function of both the type and the index, something like:

unsigned getThreadHandle(unsigned index);
unsigned getThreadIndex(unsigned handle);

And if you pass a non-thread handle to getThreadIndex an exception is
thrown.

 

[Tomás Ó hÉilidhe]

IF you do not want them to be mixed up you must use different types
for them, creating new types are done using class, struct, or enum. An
alternative is to use run-time checks, so instead of using indexes you
use handles (which could be unsigned ints) where each handles value is
some reversible function of both the type and the index, something
like:

unsigned getThreadHandle(unsigned index);
unsigned getThreadIndex(unsigned handle);

And if you pass a non-thread handle to getThreadIndex an exception is
thrown.

Only problem is that 3 could be a valid value for both an index and a
handle.

So far I'm still sticking with the simple struct method.