variable type class (was: std vector use question)

[bartek d]

Hello,

Regarding my previous question about a class which is used to store a
variable type vector. I tried to be more elaborate on the code.

I'd be grateful for your suggestions. Am I going in the wrong direction
with the implementation? I'm asking this because I don't have much
experience with C++. Thanks in advance.

The main problem I see with this class, is that the code which uses it
must first ask for its type before invoking the Get* methods.

typedef std::vector<int> IntVector;
typedef std::vector<float> FloatVector;
typedef std::vector<std::string> StringVector;

class RVar {
public:
//
// Possible var types.
//
enum RVarType {
UNDEFINED = 0, FLOAT, INT, STRING
};
//
// Default ctor.
//
RVar() : ptr(NULL), type(UNDEFINED) { }
//
// Copy ctor.
//
RVar(const RVar& src) : ptr(NULL), type(src.type) {
if(type == FLOAT)
ptr = new FloatVector(*static_cast<FloatVector*>(src.ptr));
else if(type = INT)
ptr = new IntVector(*static_cast<IntVector*>(src.ptr));
else if(type == STRING)
ptr = new StringVector(*static_cast<StringVector*>(src.ptr));
}
//
// Default dtor.
//
~RVar() { if(type) DoCleanup(); }
//
// Determine the type.
//
bool TypeIs(RVarType t) {
return type == t;
}
//
// Writing data accessors
//
RVar& SetFloatVector(const FloatVector& src) {
if(type) DoCleanup();
type = FLOAT;
ptr = new FloatVector(src);
return *this;
}
RVar& SetIntVector(const IntVector& src) {
if(type) DoCleanup();
type = INT;
ptr = new IntVector(src);
return *this;
}
RVar& SetStringVector(const StringVector& src) {
if(type) DoCleanup();
type = STRING;
ptr = new StringVector(src);
return *this;
}
//
// Reading data accessors
//
FloatVector& GetFloatVector()
{ return *static_cast<FloatVector*>(ptr); }
IntVector& GetIntVector()
{ return *static_cast<IntVector*>(ptr); }
StringVector& GetStringVector()
{ return *static_cast<StringVector*>(ptr); }

private:
//
// Delete data vector.
//
void DoCleanup() {
if(type == FLOAT)
delete static_cast<FloatVector*>(ptr);
else if(type == INT)
delete static_cast<IntVector*>(ptr);
else if(type == STRING)
delete static_cast<StringVector*>(ptr);
}

void *ptr;
RVarType type;
};

 

[Victor Bazarov]

Regarding my previous question about a class which is used to store a
variable type vector. I tried to be more elaborate on the code.

I'd be grateful for your suggestions. Am I going in the wrong direction
with the implementation? I'm asking this because I don't have much
experience with C++. Thanks in advance.

The main problem I see with this class, is that the code which uses it
must first ask for its type before invoking the Get* methods.

typedef std::vector<int> IntVector;
typedef std::vector<float> FloatVector;
typedef std::vector<std::string> StringVector;

class RVar {
public:
//
// Possible var types.
//
enum RVarType {
UNDEFINED = 0, FLOAT, INT, STRING
};
[...]
void *ptr;
RVarType type;
};

I'll play a devil's advocate a little. Don't get offended, it's
nothing personal...

So, what happens when somebody needs to add another type to the
set of "allowed" types? The whole class has to be changed because
it is ridden with "if-else if-else" (your garden variety "switch")
statements. Extremely inconvenient and difficult to maintain.

Can it be overcome? Yes. You need to explore templates, they
ought to be able to help. What you need to work on is minimising
of the code to be changed to add a type. When you can get to that
point, you've learned a lot about C++. [Here is the path for your
improvement study, eh?]

Bottomline is, if it works for you, of course, use it. But know
of the limitations and maintainability of the code you write. As
soon as you join a team of more than two people, the code
like your class could present a big headache.

Victor

 

[Patrick Frankenberger]

"bartek d":

Hello,

Regarding my previous question about a class which is used to store a
variable type vector. I tried to be more elaborate on the code.

I'd be grateful for your suggestions. Am I going in the wrong direction
with the implementation? I'm asking this because I don't have much
experience with C++. Thanks in advance.

What good does a class storing a variable type vector do? You can use
Boost::Any which can store any type, not just different vectors.
Still i think changing the design of the programm is a better idea in most
cases. Most problems can be solved in an easy way which doesn't need
"store-anything"-classes.

Patrick

 

[John Harrison]

Hello,

Regarding my previous question about a class which is used to store a
variable type vector. I tried to be more elaborate on the code.

I'd be grateful for your suggestions. Am I going in the wrong direction
with the implementation? I'm asking this because I don't have much
experience with C++. Thanks in advance.

I think many people would say that you that you are going in the wrong
direction with the design. Why do you need to store unrelated types in a
vector? There is almost always a better way.

The main problem I see with this class, is that the code which uses it
must first ask for its type before invoking the Get* methods.

typedef std::vector<int> IntVector;
typedef std::vector<float> FloatVector;
typedef std::vector<std::string> StringVector;

class RVar {
public:
//
// Possible var types.
//
enum RVarType {
UNDEFINED = 0, FLOAT, INT, STRING
};
//
// Default ctor.
//
RVar() : ptr(NULL), type(UNDEFINED) { }
//
// Copy ctor.
//
RVar(const RVar& src) : ptr(NULL), type(src.type) {
if(type == FLOAT)
ptr = new FloatVector(*static_cast<FloatVector*>(src.ptr));
else if(type = INT)
ptr = new IntVector(*static_cast<IntVector*>(src.ptr));
else if(type == STRING)
ptr = new StringVector(*static_cast<StringVector*>(src.ptr));
}
//
// Default dtor.
//
~RVar() { if(type) DoCleanup(); }
//
// Determine the type.
//
bool TypeIs(RVarType t) {
return type == t;
}
//
// Writing data accessors
//
RVar& SetFloatVector(const FloatVector& src) {
if(type) DoCleanup();
type = FLOAT;
ptr = new FloatVector(src);
return *this;
}
RVar& SetIntVector(const IntVector& src) {
if(type) DoCleanup();
type = INT;
ptr = new IntVector(src);
return *this;
}
RVar& SetStringVector(const StringVector& src) {
if(type) DoCleanup();
type = STRING;
ptr = new StringVector(src);
return *this;
}
//
// Reading data accessors
//
FloatVector& GetFloatVector()
{ return *static_cast<FloatVector*>(ptr); }
IntVector& GetIntVector()
{ return *static_cast<IntVector*>(ptr); }
StringVector& GetStringVector()
{ return *static_cast<StringVector*>(ptr); }

private:
//
// Delete data vector.
//
void DoCleanup() {
if(type == FLOAT)
delete static_cast<FloatVector*>(ptr);
else if(type == INT)
delete static_cast<IntVector*>(ptr);
else if(type == STRING)
delete static_cast<StringVector*>(ptr);
}

void *ptr;
RVarType type;
};

You're lacking an assignment operator, you aren't using const where it would
be appropriate, other than that I don't see any code problems. A minor point
but I'd include the if (type == UNDEFINED) case in the DoCleanup function,
that way you don't have to keep saying if (type) DoCleanup();

But its real ugly, almost certainly there is a better way. Why do you need
this?

john

 

[bartek d]

I think many people would say that you that you are going in the wrong
direction with the design. Why do you need to store unrelated types in
a vector? There is almost always a better way.
(...)

You're lacking an assignment operator, you aren't using const where it
would be appropriate, other than that I don't see any code problems. A
minor point but I'd include the if (type == UNDEFINED) case in the
DoCleanup function, that way you don't have to keep saying if (type)
DoCleanup();

But its real ugly, almost certainly there is a better way. Why do you
need this?

Certainly, there's too much "C" in my way of thinking.

I need a way to keep storage of variables a'la a symbol table, also having
default values for those.

Regards,
bartek

 

[John Harrison]

Certainly, there's too much "C" in my way of thinking.

I need a way to keep storage of variables a'la a symbol table, also having
default values for those.

OK, symbol table sounds like one of the few cases where this sort of coding
(or some other similar hack) is necessary.

john