Design problem

Code:

---

Term::Term(const Term& t) {
  std::vector<Factor*> x = t.factors;
  if (x.empty()) return;
  Factor* f;
  for (std::vector<Factor*>::iterator i = x.begin();i != x.end();++i) {
    if ((*i)->who_am_i() == Factor::FLOATING) {
      f = new Float(*((Float*)(*i)));
      factors.push_back(f);
    }
    if ((*i)->who_am_i() == Factor::FACTOR_EXPRESSION) {
      f = new Factor_Expression(*((Factor_Expression*)*i));
      factors.push_back(f);
    }
    if ((*i)->who_am_i() == Factor::FUNCTION) {
      f = new Function(*((Function*)*i));
      factors.push_back(f);
    }
    if ((*i)->who_am_i() == Factor::VARIABLE) {
      f = new Variable(*((Variable*)*i));
      factors.push_back(f);
    }
  }
}

---

Four classes (Variable, Function, Factor_Expression, and Float) derive from the class Factor.

The goal of this constructor is to create new objects as you can see and put them into a vector array. I'm sure there's a simpler way to do this, something more OOP friendly, but I can't think of it.

it would be a little short sighted to comment without knowing more about the whole structure of your app.
Can you give a synopsis of your classes, their responsibility and how they are related to other classes.
Do you know and understand object factories?

You can use dynamic_cast<>() and typeid() instead of the who_am_i() function.

You should use dynamic_cast<>() instead of the C-style casts.

Quote:

---

Do you know and understand object factories?

---

No... I haven't heard the term before.

The purpose of this function is to create a new object for each object pointer in the vector array 'factors'.

I think there should be a way to simplify this. Right now, the function checks each pointer to determine its object type, then creates a new appropriate object, and puts it into the vector array 'factors'.

So my question is, is there a way to let the object itself decide the new type of object to be produced?

I'm thinking of something like this:

Code:

---

Factor* Variable::new_object() {
  return new Factor(*this);
}

---

And doing it for each class. But I'm not sure if that's the right way to do it.

//edit: thanks, sang, i'll look into it

Perhaps this is a little better:

Code:

---

Term::Term(const Term& t) {
  std::vector<Factor*> x = t.factors;
  if (x.empty()) return;

  Float* float;
  Expression* expression;
  Function* function;
  Variable* variable;
 
  for (int i = 0;i < x.size(); ++i)
  {
    if (float = dynamic_cast<Float*>(x[i]))
      factors.push_back(new Float( *float ));

    else if (expression = dynamic_cast<Expression*>(x[i]))
      factors.push_back(new Expression( *expression ));

    else if (function = dynamic_cast<Function*>(x[i]))
      factors.push_back(new Function( *function ));
     
    else if (variable = dynamic_cast<Variable*>(x[i]))
      factors.push_back(new Variable( *variable ));

  }
}

---

EDIT: The new_object() solution could work, make it virtual and you can get rid of all the if-statements!

read this carefully. It explains a lot. If you get lost then start again and reread it. You will have some new ideas after reading thru this sample chapter of Andrei Alexandrescus Modern cpp design.

Looks interesting; I've printed the pages and I'll read them later.