Hi,

I am trying to simplify initialising objects from file and writing objects to file by overloading the "<<" and ">>" operators.

I have two classes, a nesting class and the nested class, the latter comprising two floats. The nested class forms and STL vector

class beamlets{

float left,right;

...

};

class dCP{

float d,M,g,c,u;
std::vector<beamlet> beamlets;
.
.
.
};

Thus I need something like the following (I want to use iterators, if possible)

ostream& operator<< (ostream &out, const dCP &dcp)
	{
	    // Since operator<< is a friend of the dicomCP class, we can access
	    // dicomCP's members directly.
	  out <<  dcp.d << "\n"  << dcp.M <<  dcp.g << "\n" << 
	      dcp.c << "\n"  <<
	      dcp.u << "\n";
	  return out;
	}

ostream & operator << (ostream &out, const std::vector<beamlet> &b)
  std::ostream out1,out2;
for (b::const_iterator it=b.begin();it!=b.end();++b){
	    if (it!=b.end()-2) out1 << it.left << "," << it.right << ",\n";
	    else out2 << it.left << "," << it.right << "\n";
	    out << out1 << out2;
	    return out;
	}

The first operator output the "header" information and the second outputs the body of the STL vector. I then need to chain these operators to output data from a file into the object... something like

// dCP object d contains beamlets
dCP d;

std::ofstream fileOut(file_out.c_str());

fileOut << d << d.beamlets;

Am I going about this the correct way? If not is there a better (more efficient, robust or elegant way) of achieving this?

Many thanks

Mark

Why do you need two operators <<? I mean if you put the code for the second one into the first one, you will only need to do "fileOut << d;". Moreover, the second call "<< d.beamlets" is not going to work because beamlets is a private member of the DCP class. If you need the second << operator (for the vector of beamlet), then just call it inside the first one, i.e. "return out << dcp.beamlets;".

Then, this piece of code seems really weird to me:

if (it!=b.end()-2) out1 << it.left << "," << it.right << ",\n";
	    else out2 << it.left << "," << it.right << "\n";

I don't think that "b.end()-2" is meaningful at all, most probably, it is undefined behaviour, so try to avoid it. Use indexing instead of operators to check where you are in the array. And what is this piece of code supposed to accomplish? The comma at the end of the first line, in ",\n", is useless, and this way, you don't need this conditional at all.

Other then that, it seem fine. BTW, these are NOT nested classes, these are just two friend classes. But maybe beamlet should be nested in DCP.

Readability >>> clever tricks

This code is painful:

ostream & operator << (ostream &out, const std::vector<beamlet> &b)
  std::ostream out1,out2;
for (b::const_iterator it=b.begin();it!=b.end();++b){
	    if (it!=b.end()-2) out1 << it.left << "," << it.right << ",\n";
	    else out2 << it.left << "," << it.right << "\n";
	    out << out1 << out2;
	    return out;
	}

This code does exactly the same thing, and it is much plainer what is happening:

ostream& operator<<( ostream &out, const std::vector<beamlet>& b )
{
    unsigned int i = b.size() - 2;
    out << b[i].left << "," << b[i].right << endl;
    for( i=0; i<b.size(); i++ )
    {
        if( i == b.size() -2 )
            continue;
        out << b[i].left << "," << b[i].right << "," << endl;
    }
    return out;
}

I'd like to make a few points

1. When using a std::vector, there is no advantage to using an iterator over indexing to the member. The vector container can access members by index in constant time. I think indexing looks much cleaner, because the syntax for iterators in for loops is pure hell. Now, if you are iterating through a list, you should use an iterator.

2. cout << "\n"; is not guranteed to flush the output stream. endl is guranteed to flush the output stream, and it is more explicit anyway. I highly recommend using endl with c++ ostreams. It's a very clear way of expressing what is happening using a reserved keyword. "\n" can be easy to miss when skimming code.

3. Do you really want to put the second to last member of your beamlet vector at the beginning of your output stream? This seems like extremely strange logic.

4. No one is going to be impressed when you don't use line breaks for conditionals and loops. Its incredibly more difficult to read, and the code feels more cluttered. Use linebreaks. This is not even a suggestion. Just do it. I also reccommend that you put curly braces on their own lines. There is no real advantage to producing compact code in this day and age.

This article has been dead for over six months. Start a new discussion instead.