Re: Is this class exception safe
On Nov 11, 4:43 pm, digz <Digvijo...@gmail.com> wrote:
On Nov 11, 9:54 pm, terminator <farid.mehr...@gmail.com> wrote:
On Nov 11, 5:42 am, digz <Digvijo...@gmail.com> 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;
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 ?
in order to access the union members one extra level of indirection/
encapsulation was deviced in your code and I guess this does not
increase readability nor maitenability of yor code ,So I decided to
use an anonymous union and increase readability to some extent ,but
since we were going to define a copy constructor ,I found it hard to
make an anonymous union copy constructible.That is because we can not
make sure that the alignment of all union members is the same so the
starting address of the union would be source of question ,moreover
theoretically the largest member is also not absolutely determinable.
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).*/
you can see that the 'union unionType' and anonymous union have
similar structures except for the '_cache_t' member in the later. So
There is a good chance for the '_cache_t' member to have the same
size ,alignemnt and address as the containing anonymous union.this way
when defining the copy constructor you do not need to consider all the
case you just write:
cacheSType( const cacheSType& rhs):
_cache_t(rhs._cache_t),
_data_t(rhs._data_t){
if(_data_T==STRING)
getstr(rhs.cptr_);
};
you could also use accessor functions to union members instead of
using an anonymous union but I just did not like it.
In either case you did not need to define constructors for the union
in this context and that is what I wanted to avoid.
regards,
FM.