passing in blank ofstream

[rhymejerky]

Hi,

I have a tree and each node has an int, lf and rt ptr. I want to reuse traverse() so I can use it to delete and write the int to a file. Below is the code. For the write function, I have to pass in an ofstream, so traverse can write to it. I don't need that for remove_all. Is there a way that I can pass in an empty ofstream from remove_all?

int Roster::write_file(string _file) {

 ofstream out(_file.c_str());

 if (!out) {
  return 1;
 }

 traverse(root, Roster::SAVE, out);

 return 0;
}

void Roster::remove_all() {

 ofstream out("aa");

 traverse(root, Roster::DELETE, out);

 return;
}

void Roster::traverse(PlayerRecord *pr, Roster::ACTION action, ofstream &out) {

 if (pr) {
  if (pr->get_left()) {
   traverse(pr->get_left(), action, out);
  }

  if (pr->get_right()) {
   traverse(pr->get_right(), action, out);
  }

  switch (action) {
   case Roster::DELETE:  delete pr;
               break;
   case Roster::SAVE:   out<<pr->print_int()<<endl
	           break;
  }

 }

 return;
}

[ne555]

Use a functor.

class Action{
public:
  virtual void operator()( tree *ptr ) = 0;
};

class Delete: public Action{
public:
  void operator()( tree *ptr ){
    delete ptr;
  }
};

class Save: public Action{
private:
  std::ofstream out;
public: 
  Save(const char *filename): out(filename){}
  void operator()( tree *ptr ){
    out << ptr->value() << std::endl;
  }
};

void traverse( tree *root, Action &a){
  //...
  a(root);
  //...
}

You may want different traverses too ( pre, in, post order )
Instead of a base class, you could use templates

[hamsterman]

You could construct an empty fstream:
traverse(root, Roster::DELETE, std::ofstream());

or

You could have traverse take an ofstream* and pass null:
traverse(root, Roster::DELETE, 0);

void Roster::traverse(PlayerRecord *pr, Roster::ACTION action, ofstream *out)

(*out)<<pr->print_int()<<endl


But the problem is that you have one function that does two unrelated things. What ne555 wrote is a good solution. If you find that hard to understand, write two functions traverse_and_delete(PlayerRecord*) and traverse_and_save(PlayerRecord*, ostream&).

[ne555]

To delete a tree you just need to do delete root;.

An let the destructor of the node to handle the death of its sons.

~node(){
  delete left;
  delete right;
}

[rhymejerky]

Thanks ne555, letting the destructor does the work is great.

hamsterman: In school, the professor always says to group code that have similar function. The traverse_delete and traverse_save both traverse the tree. That is why I want to group them together. In real work environment, is it better to have the 2 separated?

[hamsterman]

It is best to do what ne555 did.

It is good to group similar code, but you should also have your functions do one thing only. If you tried to name your traverse() by what it really does, it would be called traverse_and_save_or_delete(). Those are clearly two unrelated functions. And you're not saving any code this way since you still have to write write_file() and remove_all() functions.