Is this class exception safe

[digz]

Hi ,
I am trying to write a class that encapsulates a Union within it , I
intend the code to be exception safe.
can someone please review it and tell me if I have done the right
thing , are there any obvious memory leaks or bad design , inability
to cover most likely failure paths etc..?.

Thanks in advance
Digz
-------------------

#include<exception>
enum datatype { INVALID=-1, INTEGER=1, STRING, DOUBLE };

struct cacheSType
{
private:
union cacheUType
{
int i_;
double d_;
char* cptr_;
cacheUType():d_(-999){}
cacheUType(int i):i_(i){}
cacheUType(double d):d_(d){}
cacheUType(char* c):cptr_(c){}
};
typedef cacheUType unionType;
datatype _data_t;
unionType _cache_t;

public:
cacheSType():_data_t(INVALID){}
cacheSType(int i): _data_t(INTEGER), _cache_t(i){}
cacheSType(double d): _data_t(DOUBLE), _cache_t(d){}
cacheSType(const char* c ): _data_t(STRING){
try {
_cache_t.cptr_ = new char[strlen(c) + 1 ];
strcpy(_cache_t.cptr_, c);
}
catch( const std::bad_alloc &oom ){
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

cacheSType( const cacheSType& rhs) {
try {
if ( rhs._data_t == STRING ) {
_cache_t.cptr_ = new
char[strlen(rhs._cache_t.cptr_) + 1];
strcpy(_cache_t.cptr_, rhs._cache_t.cptr_);
}
else {
_cache_t = rhs._cache_t;
}
_data_t = rhs._data_t;
}
catch( const std::bad_alloc &oom) {
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

cacheSType& operator=(const cacheSType& rhs) {
if ( &rhs != this ) {
if ( rhs._data_t != STRING ) {
if ( _data_t == STRING)
delete [] _cache_t.cptr_;
_cache_t = rhs._cache_t;
}
else{
if ( _data_t == STRING )
delete [] _cache_t.cptr_;
try {
_cache_t.cptr_= new
char[strlen(rhs._cache_t.cptr_) + 1];
}
catch( const std::bad_alloc &oom) {
cout << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
strcpy( _cache_t.cptr_, rhs._cache_t.cptr_);
}
_data_t = rhs._data_t;
}
return *this;
}

operator int () {
if ( _data_t == INTEGER )
return _cache_t.i_;
throw std::bad_cast(); //cannot return anything sensible
if not int
}

operator double () {
if ( _data_t == DOUBLE )
return _cache_t.d_;
throw std::bad_cast();
}

operator const char* () {
if ( _data_t == STRING )
return _cache_t.cptr_;
throw std::bad_cast();
}

~cacheSType(){
if ( _data_t == STRING )
delete [] _cache_t.cptr_;
}

}

[Kai-Uwe Bux]

digz wrote:

Hi ,
I am trying to write a class that encapsulates a Union within it , I
intend the code to be exception safe.
can someone please review it and tell me if I have done the right
thing , are there any obvious memory leaks or bad design , inability
to cover most likely failure paths etc..?.

Thanks in advance
Digz
-------------------

#include<exception>
enum datatype { INVALID=-1, INTEGER=1, STRING, DOUBLE };

struct cacheSType
{
private:
union cacheUType
{
int i_;
double d_;
char* cptr_;
cacheUType():d_(-999){}
cacheUType(int i):i_(i){}
cacheUType(double d):d_(d){}
cacheUType(char* c):cptr_(c){}
};
typedef cacheUType unionType;

Is there a reason to use a typedef instead of just saying

union unionType {...

datatype _data_t;
unionType _cache_t;

public:
cacheSType():_data_t(INVALID){}
cacheSType(int i): _data_t(INTEGER), _cache_t(i){}
cacheSType(double d): _data_t(DOUBLE), _cache_t(d){}
cacheSType(const char* c ): _data_t(STRING){
try {
_cache_t.cptr_ = new char[strlen(c) + 1 ];
strcpy(_cache_t.cptr_, c);
}
catch( const std::bad_alloc &oom ){
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}

I am not sure whether catch(...){throw;} does anything. It seems to just
rethrow an exception that otherwise would have propagates up the stack
anyway.

}

cacheSType( const cacheSType& rhs) {
try {
if ( rhs._data_t == STRING ) {
_cache_t.cptr_ = new
char[strlen(rhs._cache_t.cptr_) + 1];
strcpy(_cache_t.cptr_, rhs._cache_t.cptr_);
}
else {
_cache_t = rhs._cache_t;
}
_data_t = rhs._data_t;
}
catch( const std::bad_alloc &oom) {
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

cacheSType& operator=(const cacheSType& rhs) {
if ( &rhs != this ) {
if ( rhs._data_t != STRING ) {
if ( _data_t == STRING)
delete [] _cache_t.cptr_;
_cache_t = rhs._cache_t;
}
else{
if ( _data_t == STRING )
delete [] _cache_t.cptr_;
try {
_cache_t.cptr_= new
char[strlen(rhs._cache_t.cptr_) + 1];
}
catch( const std::bad_alloc &oom) {
cout << oom.what() << endl;
throw;
}
catch(...) {
throw;
}

If the above throws, you have already deleted the current string. The object
is then in an inconsistent state since the _cache_t says it's a string, but
the string-field points to deallocated memory. In that sense, the
assignment operator is not exception safe.
Also, the logic of the assignment operator is hard to follow (after all,
there are 9 possible combinations of _data_t and rhs._data_t to consider).
For clarity and exception safety, you should consider using the copy-swap
idiom.

strcpy( _cache_t.cptr_, rhs._cache_t.cptr_);
}
_data_t = rhs._data_t;
}
return *this;
}

operator int () {
if ( _data_t == INTEGER )
return _cache_t.i_;
throw std::bad_cast(); //cannot return anything sensible
if not int
}

operator double () {
if ( _data_t == DOUBLE )
return _cache_t.d_;
throw std::bad_cast();
}

operator const char* () {
if ( _data_t == STRING )
return _cache_t.cptr_;
throw std::bad_cast();
}

~cacheSType(){
if ( _data_t == STRING )
delete [] _cache_t.cptr_;
}

}

Best

Kai-Uwe Bux

[James Kanze]

On Nov 11, 4:00 am, Kai-Uwe Bux <[e-mail]:

digz wrote:

[...]

Also, the logic of the assignment operator is hard to follow
(after all, there are 9 possible combinations of _data_t and
rhs._data_t to consider). For clarity and exception safety,
you should consider using the copy-swap idiom.

In this case, definitely. I might add that you can call
std::swap on his union, and the results are guaranteed to be no
throw (since a union will have a trivial copy constructor,
destructor and assignment operator, unless the user defines them
himself). In other words, he doesn't have to analyse the type
contained by the union when doing the swap.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[terminator]

On Nov 11, 5:42 am, digz <[e-mail]:

Hi ,
I am trying to write a class that encapsulates a Union within it , I
intend the code to be exception safe.
can someone please review it and tell me if I have done the right
thing , are there any obvious memory leaks or bad design , inability
to cover most likely failure paths etc..?.

Thanks in advance
Digz
-------------------

#include<exception>
enum datatype { INVALID=-1, INTEGER=1, STRING, DOUBLE };

struct cacheSType
{
private:
union cacheUType
{
int i_;
double d_;
char* cptr_;
cacheUType():d_(-999){}
cacheUType(int i):i_(i){}
cacheUType(double d):d_(d){}
cacheUType(char* c):cptr_(c){}
};
typedef cacheUType unionType;
datatype _data_t;
unionType _cache_t;

public:
cacheSType():_data_t(INVALID){}
cacheSType(int i): _data_t(INTEGER), _cache_t(i){}
cacheSType(double d): _data_t(DOUBLE), _cache_t(d){}
cacheSType(const char* c ): _data_t(STRING){
try {
_cache_t.cptr_ = new char[strlen(c) + 1 ];
strcpy(_cache_t.cptr_, c);
}
catch( const std::bad_alloc &oom ){
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

cacheSType( const cacheSType& rhs) {
try {
if ( rhs._data_t == STRING ) {
_cache_t.cptr_ = new
char[strlen(rhs._cache_t.cptr_) + 1];
strcpy(_cache_t.cptr_, rhs._cache_t.cptr_);
}
else {
_cache_t = rhs._cache_t;
}
_data_t = rhs._data_t;
}
catch( const std::bad_alloc &oom) {
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

I would write it this way:

struct cacheSType{
private:
union unionType{
int i_;
double d_;
char* cptr_;
};

union{//anonymous union is an object itself
unionType _cache_t;//for copying only
int i_;
double d_;
char* cptr_;
};/*all members belong to nesting block(struct cacheSType).*/

datatype _data_t;

cacheSType():_data_t(INVALID){}
cacheSType(int i): _data_t(INTEGER), _cache_t(i){}
cacheSType(double d): _data_t(DOUBLE), _cache_t(d){}
cacheSType(const char* c ): _data_t(STRING){ getstr(c); };
cacheSType( const cacheSType& rhs):
_cache_t(rhs._cache_t),
_data_t(rhs._data_t){
if(_data_T==STRING)
getstr(rhs.cptr_);
};

cacheSType& operator=(const cacheSType& rhs);//defined via copy-
swap.

~cacheSType(){
if ( _data_t == STRING )
delete [] cptr_;
//just to make sure invalid data wont be used:
_data_t=INVALID;
cptr_=NULL;
};

void getstr(const char* c);//handles memory allocation

}

regards,
FM.

[digz]

On Nov 11, 7:36 pm, James Kanze <[e-mail]:

On Nov 11, 4:00 am, Kai-Uwe Bux <[e-mail]:

digz wrote:

[...]

Also, the logic of the assignment operator is hard to follow
(after all, there are 9 possible combinations of _data_t and
rhs._data_t to consider). For clarity and exception safety,
you should consider using the copy-swap idiom.

In this case, definitely. I might add that you can call
std::swap on his union, and the results are guaranteed to be no
throw (since a union will have a trivial copy constructor,
destructor and assignment operator, unless the user defines them
himself). In other words, he doesn't have to analyse the type
contained by the union when doing the swap.

Ok I read up the Copy and Swap Idiom , and now my assigment operator
looks like this :
Is this correct ?

cacheSType& cacheSType::operator=(const cacheSType& rhs) {
if ( &rhs != this )
{
cacheSType tmp(rhs);
std::swap(tmp._cache_t, cache_t);
data_t = rhs.data_t;
}
return *this;
}

[digz]

On Nov 11, 9:54 pm, terminator <[e-mail]:

On Nov 11, 5:42 am, digz <[e-mail]:

Hi ,
I am trying to write a class that encapsulates a Union within it , I
intend the code to be exception safe.
can someone please review it and tell me if I have done the right
thing , are there any obvious memory leaks or bad design , inability
to cover most likely failure paths etc..?.

Thanks in advance
Digz
-------------------

#include<exception>
enum datatype { INVALID=-1, INTEGER=1, STRING, DOUBLE };

struct cacheSType
{
private:
union cacheUType
{
int i_;
double d_;
char* cptr_;
cacheUType():d_(-999){}
cacheUType(int i):i_(i){}
cacheUType(double d):d_(d){}
cacheUType(char* c):cptr_(c){}
};
typedef cacheUType unionType;
datatype _data_t;
unionType _cache_t;

public:
cacheSType():_data_t(INVALID){}
cacheSType(int i): _data_t(INTEGER), _cache_t(i){}
cacheSType(double d): _data_t(DOUBLE), _cache_t(d){}
cacheSType(const char* c ): _data_t(STRING){
try {
_cache_t.cptr_ = new char[strlen(c) + 1 ];
strcpy(_cache_t.cptr_, c);
}
catch( const std::bad_alloc &oom ){
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

cacheSType( const cacheSType& rhs) {
try {
if ( rhs._data_t == STRING ) {
_cache_t.cptr_ = new
char[strlen(rhs._cache_t.cptr_) + 1];
strcpy(_cache_t.cptr_, rhs._cache_t.cptr_);
}
else {
_cache_t = rhs._cache_t;
}
_data_t = rhs._data_t;
}
catch( const std::bad_alloc &oom) {
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

I would write it this way:

struct cacheSType{
private:
union unionType{
int i_;
double d_;
char* cptr_;
};

union{//anonymous union is an object itself
unionType _cache_t;//for copying only
int i_;
double d_;
char* cptr_;
};/*all members belong to nesting block(struct cacheSType).*/

Dont understand why you did this ? What purpose does the anonymous
union serve ?

datatype _data_t;

cacheSType():_data_t(INVALID){}
cacheSType(int i): _data_t(INTEGER), _cache_t(i){}
cacheSType(double d): _data_t(DOUBLE), _cache_t(d){}
cacheSType(const char* c ): _data_t(STRING){ getstr(c); };
cacheSType( const cacheSType& rhs):
_cache_t(rhs._cache_t),
_data_t(rhs._data_t){
if(_data_T==STRING)
getstr(rhs.cptr_);
};

cacheSType& operator=(const cacheSType& rhs);//defined via copy-
swap.

~cacheSType(){
if ( _data_t == STRING )
delete [] cptr_;
//just to make sure invalid data wont be used:
_data_t=INVALID;
cptr_=NULL;
};

void getstr(const char* c);//handles memory allocation

}

Thx
Digz

[digz]

On Nov 11, 10:31 pm, digz <[e-mail]:

On Nov 11, 7:36 pm, James Kanze <[e-mail]:

On Nov 11, 4:00 am, Kai-Uwe Bux <[e-mail]:

digz wrote:

[...]

Also, the logic of the assignment operator is hard to follow
(after all, there are 9 possible combinations of _data_t and
rhs._data_t to consider). For clarity and exception safety,
you should consider using the copy-swap idiom.

In this case, definitely. I might add that you can call
std::swap on his union, and the results are guaranteed to be no
throw (since a union will have a trivial copy constructor,
destructor and assignment operator, unless the user defines them
himself). In other words, he doesn't have to analyse the type
contained by the union when doing the swap.

Ok I read up the Copy and Swap Idiom , and now my assigment operator
looks like this :
Is this correct ?

cacheSType& cacheSType::operator=(const cacheSType& rhs) {
if ( &rhs != this )
{
cacheSType tmp(rhs);
std::swap(tmp._cache_t, _cache_t);

std::swap(tmp._data_t, _data_t);

}
return *this;

}- Hide quoted text -

Added the extra line std::swap(tmp._data_t, _data_t ) in the
assignment operator
Had to swap the _data_t otherwise tmp was in an inconsistent state ,
and the tmp destructor call was segfaulting
If there is still something wrong please let me know
Thanks very much for all your comments.
--Digz

[Kai-Uwe Bux]

digz wrote:

On Nov 11, 10:31 pm, digz <[e-mail]:

>On Nov 11, 7:36 pm, James Kanze <[e-mail]:

On Nov 11, 4:00 am, Kai-Uwe Bux <[e-mail]:

digz wrote:

[...]

Also, the logic of the assignment operator is hard to follow
(after all, there are 9 possible combinations of _data_t and
rhs._data_t to consider). For clarity and exception safety,
you should consider using the copy-swap idiom.

In this case, definitely. I might add that you can call
std::swap on his union, and the results are guaranteed to be no
throw (since a union will have a trivial copy constructor,
destructor and assignment operator, unless the user defines them
himself). In other words, he doesn't have to analyse the type
contained by the union when doing the swap.

Ok I read up the Copy and Swap Idiom , and now my assigment operator
looks like this :
Is this correct ?

cacheSType& cacheSType::operator=(const cacheSType& rhs) {
if ( &rhs != this )
{
cacheSType tmp(rhs);
std::swap(tmp._cache_t, _cache_t);

std::swap(tmp._data_t, _data_t);

> }
return *this;

This looks right.

BTW: the test for self-assignment is not needed for correctness. Whether it
increases or decreases performance depends on how frequent self-assignment
is in the application.
Also: you could move the two swap lines to a swap friend function

friend
void swap ( cacheSType & rhs, cacheSType lhs ) {
std::swap( lhs._cache_t, rhs._cache_t );
std::swap( lhs._data_t, rhs._data_t );
}

and then call that in the assignment operator. This swap() could then be
used in other places instead of std::swap<cacheSType>, which would call
your assignment-operator three times resulting in 6 member-swaps as opposed
to only two.

>}- Hide quoted text -

Added the extra line std::swap(tmp._data_t, _data_t ) in the
assignment operator
Had to swap the _data_t otherwise tmp was in an inconsistent state ,
and the tmp destructor call was segfaulting
If there is still something wrong please let me know

Best

Kai-Uwe Bux

[James Kanze]

Kai-Uwe Bux wrote:

[...]

BTW: the test for self-assignment is not needed for correctness.

In general (there are probably exceptions), the need for a test
for self-assignment is often a sign that the assignment operator
is not excetpion safe (but as you point out, it's not necessary
here).

Whether it increases or decreases performance depends on how
frequent self-assignment is in the application.

Also: you could move the two swap lines to a swap friend function

friend
void swap ( cacheSType & rhs, cacheSType lhs ) {
std::swap( lhs._cache_t, rhs._cache_t );
std::swap( lhs._data_t, rhs._data_t );
}

and then call that in the assignment operator. This swap()
could then be used in other places instead of
std::swap<cacheSType>, which would call your
assignment-operator three times resulting in 6 member-swaps as
opposed to only two.

I think it would be more idiomatic to have a swap member
fucntion, and specialize std::swap to use it. I usually assume
that if a class does not have a swap member function, std::swap
will be done using the "standard" algorithm, with copy
construction and assignment, and don't use the swap idiom if my
class contains members of class type which don't have a swap
member function.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[Kai-Uwe Bux]

James Kanze wrote:

Kai-Uwe Bux wrote:

[...]

>BTW: the test for self-assignment is not needed for correctness.

In general (there are probably exceptions), the need for a test
for self-assignment is often a sign that the assignment operator
is not excetpion safe (but as you point out, it's not necessary
here).

>Whether it increases or decreases performance depends on how
frequent self-assignment is in the application.

>Also: you could move the two swap lines to a swap friend function

> friend
void swap ( cacheSType & rhs, cacheSType lhs ) {
std::swap( lhs._cache_t, rhs._cache_t );
std::swap( lhs._data_t, rhs._data_t );
}

>and then call that in the assignment operator. This swap()
could then be used in other places instead of
std::swap<cacheSType>, which would call your
assignment-operator three times resulting in 6 member-swaps as
opposed to only two.

I think it would be more idiomatic to have a swap member
fucntion, and specialize std::swap to use it.

I disagree (or at least I would contend that what is idiomatic in this area
is shifting). I just put a swap() function into the same namespace as the
class. Then name-lookup will find it. Note that the next version of the
standard makes this (and not a swap member function) one of the two ways to
satisfy the swappable requirement.

I usually assume
that if a class does not have a swap member function, std::swap
will be done using the "standard" algorithm, with copy
construction and assignment,

That assumption could also be correct for classes that have a swap member.
You are heavily relying on (local) coding conventions should you assume
that std::swap is specialized for classes that have a swap member.

I follow the convention not to use std::swap, but swap (usually after making
std::swap visible via "using"). This way, I can swap everything swappable.
I think next version of the standard will ensure that standard algorithm
will use swap and not std::swap so that I specializing std::swap is
superfluous.

and don't use the swap idiom if my
class contains members of class type which don't have a swap
member function.

I just assume that members are swappable. _If_ the swap is no-throw for
_all_ members, then so is mine (and copy-swap-assignment will be exception
safe).
Best

Kai-Uwe Bux

[Kai-Uwe Bux]

Alf P. Steinbach wrote:

* Kai-Uwe Bux:

>>
I follow the convention not to use std::swap, but swap (usually after
making std::swap visible via "using"). This way, I can swap everything
swappable. I think next version of the standard will ensure that standard
algorithm will use swap and not std::swap so that I specializing
std::swap is superfluous.

Well, currently it's formally invalid to specialize any function in
namespace std: classes yes, functions no, njet, nein, nei, nope. Very
stupid rule. I just ignore it for my own private small programs. ;-)

I thought so, too; and I was about to mention it. However, then I looked up
the wording:

A program may add template specializations for any standard library
template to namespace std.

Now, I used to think that one cannot provide alternative specializations for
function templates (all one can do is to add overloads). Then, I used the
pdf-reader to search for specializations and found that the standard
acknowledges the existence of function template specializations (oops). At
that point, I was not sure anymore whether my previous thinking was correct
(it's kind of late). That's why I dropped the issue.
Best

Kai-Uwe Bux

[Andre Kostur]

"Alf P. Steinbach" <al***@start.nowrote in
news:13*************@corp.supernews.com:

* Kai-Uwe Bux:

>>
I follow the convention not to use std::swap, but swap (usually after
making std::swap visible via "using"). This way, I can swap
everything swappable. I think next version of the standard will
ensure that standard algorithm will use swap and not std::swap so
that I specializing std::swap is superfluous.

Well, currently it's formally invalid to specialize any function in
namespace std: classes yes, functions no, njet, nein, nei, nope. Very
stupid rule. I just ignore it for my own private small programs. ;-)

Chapter and verse?

I found 17.4.3.1.1 which specifically mentions: "A program may add template
specializations for any standard library template to namespace std." This
seems to specifically allow one to specialize std::swap for one's own
classes.

[James Kanze]

On Nov 12, 2:52 pm, "Alf P. Steinbach" <[e-mail]:

* Andre Kostur:

"Alf P. Steinbach" <[e-mail] in
news:13*************@corp.supernews.com:

* Kai-Uwe Bux:
I follow the convention not to use std::swap, but swap (usually after
making std::swap visible via "using"). This way, I can swap
everything swappable. I think next version of the standard will
ensure that standard algorithm will use swap and not std::swap so
that I specializing std::swap is superfluous.
Well, currently it's formally invalid to specialize any function in
namespace std: classes yes, functions no, njet, nein, nei, nope. Very
stupid rule. I just ignore it for my own private small programs. ;-)

Chapter and verse?

I found 17.4.3.1.1 which specifically mentions: "A program
may add template specializations for any standard library
template to namespace std." This seems to specifically
allow one to specialize std::swap for one's own classes.

That only covers template specializations.

Here's a template specialization of std::swap:

namespace std
{
template<void swap( MyClass& a, MyClass& b ) { a.swap( b ) }
}

and that is OK,

And that's what we're talking about.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

Kai-Uwe Bux wrote:

James Kanze wrote:

[...]

The question is not how a member swap is implemented. The
question is whether a swap member function indicates that
std::swap<is properly specialized. (Also, with regard to the
issues discusses elsethread, I would wonder how many times
std::swap<is improperly overloaded:-)

As I said, in my own classes, I use the member function for
class types; I don't suppose std::swap is properly specialized.

I see. For the template stuff that I am dealing with, this
would be the most inconvenient convention.

This is probably the difference. The only times I use templates
are in very low level stuff. In which case, the only class
types I have to deal with are my own, or those in the standard
library; there are no other libraries below me. And of course,
the class types in my own code and in the standard library all
have a member swap. Using it (and explicitly std::swap for the
non class types) avoids the name lookup issue of calling swap
from within a function named swap.

I can see, however, that it is probably the most
convenient for you (and it makes the least assumptions).

In the concrete situations I've had to deal with to date:-).

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[ExcessPhase]

On Nov 10, 6:42 pm, digz <[e-mail]:

Hi ,
I am trying to write a class that encapsulates a Union within it , I
intend the code to be exception safe.
can someone please review it and tell me if I have done the right
thing , are there any obvious memory leaks or bad design , inability
to cover most likely failure paths etc..?.

Thanks in advance
Digz
-------------------

what is wrong with boost::any
?
See also:

http://www.boost.org/doc/html/any.html

[digz]

On Nov 16, 7:27 am, [e-mail] wrote:

On Nov 10, 6:42 pm, digz <[e-mail]:

Hi ,
I am trying to write a class that encapsulates a Union within it , I
intend the code to be exception safe.
can someone please review it and tell me if I have done the right
thing , are there any obvious memory leaks or bad design , inability
to cover most likely failure paths etc..?.

Thanks in advance
Digz
-------------------

what is wrong with boost::any
?
See also:

http://www.boost.org/doc/html/any.html

I wanted something lightweight, I am always going to store ints,
doubles or strings, (if i ever need to store bigger objects i will
probably serialize them as strings and deserialize while retrieving ),
have to write lot of code around boost::any to identify types at
runtime and dispatch to the right method...

boost::variant another option( types fixed at compile time) also leads
to huge binaries, i have to write the visitation code.., compiling
requires -ftemplate-depth 50 .something which co workers do not like
coz increased template depth .mean crazy build times..for the source
tree...
to cut a long story short keep it nice and simple.!

[terminator]

On Nov 11, 3:54 pm, terminator <[e-mail]:

On Nov 11, 5:42 am, digz <[e-mail]:

Hi ,
I am trying to write a class that encapsulates a Union within it , I
intend the code to be exception safe.
can someone please review it and tell me if I have done the right
thing , are there any obvious memory leaks or bad design , inability
to cover most likely failure paths etc..?.

Thanks in advance
Digz
-------------------

#include<exception>
enum datatype { INVALID=-1, INTEGER=1, STRING, DOUBLE };

struct cacheSType
{
private:
union cacheUType
{
int i_;
double d_;
char* cptr_;
cacheUType():d_(-999){}
cacheUType(int i):i_(i){}
cacheUType(double d):d_(d){}
cacheUType(char* c):cptr_(c){}
};
typedef cacheUType unionType;
datatype _data_t;
unionType _cache_t;

public:
cacheSType():_data_t(INVALID){}
cacheSType(int i): _data_t(INTEGER), _cache_t(i){}
cacheSType(double d): _data_t(DOUBLE), _cache_t(d){}
cacheSType(const char* c ): _data_t(STRING){
try {
_cache_t.cptr_ = new char[strlen(c) + 1 ];
strcpy(_cache_t.cptr_, c);
}
catch( const std::bad_alloc &oom ){
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

cacheSType( const cacheSType& rhs) {
try {
if ( rhs._data_t == STRING ) {
_cache_t.cptr_ = new
char[strlen(rhs._cache_t.cptr_) + 1];
strcpy(_cache_t.cptr_, rhs._cache_t.cptr_);
}
else {
_cache_t = rhs._cache_t;
}
_data_t = rhs._data_t;
}
catch( const std::bad_alloc &oom) {
cerr << oom.what() << endl;
throw;
}
catch(...) {
throw;
}
}

I would write it this way:

struct cacheSType{
private:
union unionType{
int i_;
double d_;
char* cptr_;
};

union{//anonymous union is an object itself
unionType _cache_t;//for copying only
int i_;
double d_;
char* cptr_;
};/*all members belong to nesting block(struct cacheSType).*/

datatype _data_t;

cacheSType():_data_t(INVALID){}
cacheSType(int i): _data_t(INTEGER), _cache_t(i){}

//oops I mean:
cacheSType(int i): _data_t(INTEGER), i_(i){}

cacheSType(double d): _data_t(DOUBLE), _cache_t(d){}

//and:
cacheSType(double d): _data_t(DOUBLE), d_(d){}

cacheSType(const char* c ): _data_t(STRING){ getstr(c); };
cacheSType( const cacheSType& rhs):
_cache_t(rhs._cache_t),
_data_t(rhs._data_t){
if(_data_T==STRING)
getstr(rhs.cptr_);
};

cacheSType& operator=(const cacheSType& rhs);//defined via copy-
swap.

~cacheSType(){
if ( _data_t == STRING )
delete [] cptr_;
//just to make sure invalid data wont be used:
_data_t=INVALID;
cptr_=NULL;
};

void getstr(const char* c);//handles memory allocation

}

regards,
FM.- Hide quoted text -

- Show quoted text -

regards,
FM.

[terminator]

On Nov 15, 3:05 pm, James Kanze <[e-mail]:

Kai-Uwe Bux wrote:

James Kanze wrote:

[...]

>The question is not how a member swap is implemented. The
>question is whether a swap member function indicates that
>std::swap<is properly specialized. (Also, with regard to the
>issues discusses elsethread, I would wonder how many times
>std::swap<is improperly overloaded:-)
As I said, in my own classes, I use the member function for
class types; I don't suppose std::swap is properly specialized.

I see. For the template stuff that I am dealing with, this
would be the most inconvenient convention.

This is probably the difference. The only times I use templates
are in very low level stuff. In which case, the only class
types I have to deal with are my own, or those in the standard
library; there are no other libraries below me. And of course,
the class types in my own code and in the standard library all
have a member swap. Using it (and explicitly std::swap for the
non class types) avoids the name lookup issue of calling swap
from within a function named swap.

I can see, however, that it is probably the most
convenient for you (and it makes the least assumptions).

In the concrete situations I've had to deal with to date:-).

--
James Kanze (GABI Software) email:[e-mail]
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

forgive me for not reading the whole argument about ways to define
swap,but I ask why not to force standard to be like this:

namespace std{
template <swapable T>
void swap(T& l,T&r){
if(&l!==&r)l.swap(r);
};

template<unswapable T>
void swap(T& l,T&r){
TripleAssign(l,r)
};

template<intrinsic T>
void swap(T& l,T&r);

template<assignable T>
where copyable<T>
TripleAssign(T& l,T&r){
T t(l);
l=r;
r=t;
};

template<swapable T>
swap_assign(T& dest,const T& src){
T t(src);
swap(dest,t);
};
};

in the absence of cocepts three overloads can be three distinct
templates with distinct names(just as TripleAsssign).This also lets
the programer to properly decide about the swapabitily of his class.

programmer code:

const class A& A::operator=(const A& r){
swap_assign(*this,r);
};
regards,
FM.

[Ben Rudiak-Gould]

James Kanze wrote:

For that matter, you can have all three: a member swap (which
seems the most natural to me), a free-standing swap in the
ambient namespace which calls it, and a specialization of
std::swap.

It's really a shame that K&R C didn't have a swap operator. It might have
had one; I think that many machines had swap opcodes, and compilers didn't
optimize very well. It would have made a lot of sense to define x <-y to
do the same thing as the three-assignment idiom, but more efficiently. Then
this whole silly problem would never have existed. You could define
operator<-as a member or a non-member as you liked, or omit it entirely
since the default definition would be correct for most classes (far more
than the default operator= is correct for). There would be no need for the
three-assignment std::swap, so no worries about someone calling it by
accident. It's funny how much in life hinges on syntax.

-- Ben