Creating an object that is read from an input stream.

[Jason Heyes]

I want to allow objects of my class to be read from an input stream. I am
having trouble with the implementation. Here are the different approaches I
have tried:

// Version 1.0 - Default constructors
class MyClass
{
Foo foo; // foo and bar require default constructors
Bar bar;
public:
// default constructor for MyClass
MyClass() // foo and bar are default initialised
{
}

std::istream &extract(std::istream &is)
{
if (!(is >> foo))
return is;
if (!(is >> bar))
return is;
return is;
}
};

std::istream &operator>>(std::istream &is, MyClass &object)
{
return is >> object;
}

// Version 2.0 - std::istream constructor argument, default constructors and
exceptions
class MyClass
{
Foo foo; // foo and bar require default constructors
Bar bar;
public:
MyClass(std::istream &is) // foo and bar are default initialised
{
if (!(is >> foo))
throw MyException;
if (!(is >> bar))
throw MyException;
}
};

// Version 3.0 - Shared pointers
class MyClass
{
Foo foo;
Bar bar;
public:
MyClass(Foo foo_, Bar bar_) : foo(foo_), bar(bar_) { }
};

std::istream &operator>>(std::istream &is, SharedPtr<MyClass> &object_ptr)
{
SharedPtr<Foo> foo_ptr;
if (!(is >> foo_ptr))
return is;
Foo foo = *foo_ptr;

SharedPtr<Bar> bar_ptr;
if (!(is >> bar_ptr))
return is;
Bar bar = *bar_ptr;

object_ptr = new MyClass(foo, bar);
return is;
}

// Version 4.0 - Static read methods and exceptions
class MyClass
{
Foo foo
Bar bar;
public:
MyClass(Foo foo_, Bar bar_) : foo(foo_), bar(bar_) { }

static MyClass read(std::istream &is);
};

MyClass MyClass::read(std::istream &is)
{
try {
Foo foo = Foo::read(is);
Bar bar = Bar::read(is);
return MyClass(foo, bar);
} catch (FooException e) {
throw MyException(e.get_desc());
} catch (BarException e) {
throw MyException(e.get_desc());
}
}

The third version is the only approach that does not involve default
constructors or exceptions. How else can I write my class to allow it's
objects to be read from a stream? Thanks.

 

[Jason Heyes]

I wrote version 2.0 wrong. Here is what I really meant to write:

// Version 2.0 - std::istream constructor argument and exceptions
class MyClass
{
Foo foo;
Bar bar;
public:
MyClass(std::istream &is) : foo(is), bar(is)
{
}
};

This makes version 4.0 redundant.

 

[David White]

I want to allow objects of my class to be read from an input stream. I am
having trouble with the implementation. Here are the different approaches I
have tried:

// Version 1.0 - Default constructors
class MyClass
{
Foo foo; // foo and bar require default constructors
Bar bar;
public:
// default constructor for MyClass
MyClass() // foo and bar are default initialised
{
}

std::istream &extract(std::istream &is)
{
if (!(is >> foo))
return is;
if (!(is >> bar))
return is;
return is;
}
};

std::istream &operator>>(std::istream &is, MyClass &object)
{
return is >> object;

This function does an infinite recursive call. Did you mean
object.extract(is)?

}

Why not make this function a friend of the class, and let it do what the
class extract function does, and get rid of the extract function? Why take
two steps when you can take one?

// Version 2.0 - std::istream constructor argument, default constructors and
exceptions
class MyClass
{
Foo foo; // foo and bar require default constructors
Bar bar;
public:
MyClass(std::istream &is) // foo and bar are default initialised
{
if (!(is >> foo))
throw MyException;
if (!(is >> bar))
throw MyException;
}
};

With this, you can't read into an existing object.

// Version 3.0 - Shared pointers
class MyClass
{
Foo foo;
Bar bar;
public:
MyClass(Foo foo_, Bar bar_) : foo(foo_), bar(bar_) { }
};

std::istream &operator>>(std::istream &is, SharedPtr<MyClass> &object_ptr)
{
SharedPtr<Foo> foo_ptr;
if (!(is >> foo_ptr))
return is;
Foo foo = *foo_ptr;

SharedPtr<Bar> bar_ptr;
if (!(is >> bar_ptr))
return is;
Bar bar = *bar_ptr;

object_ptr = new MyClass(foo, bar);
return is;
}

All this looks like a lot of trouble.

// Version 4.0 - Static read methods and exceptions
class MyClass
{
Foo foo
Bar bar;
public:
MyClass(Foo foo_, Bar bar_) : foo(foo_), bar(bar_) { }

static MyClass read(std::istream &is);
};

MyClass MyClass::read(std::istream &is)
{
try {
Foo foo = Foo::read(is);
Bar bar = Bar::read(is);
return MyClass(foo, bar);
} catch (FooException e) {
throw MyException(e.get_desc());
} catch (BarException e) {
throw MyException(e.get_desc());
}
}

Again, you can't read into an existing object.

Of all these, version 1 (suitably modified) looks the best to me, but maybe
you have special reasons for preferring another.

DW

 

[Cy Edmunds]

I want to allow objects of my class to be read from an input stream. I am
having trouble with the implementation. Here are the different approaches I
have tried:

// Version 1.0 - Default constructors
class MyClass
{
Foo foo; // foo and bar require default constructors
Bar bar;
public:
// default constructor for MyClass
MyClass() // foo and bar are default initialised
{
}

std::istream &extract(std::istream &is)
{
if (!(is >> foo))
return is;
if (!(is >> bar))
return is;
return is;
}
};

std::istream &operator>>(std::istream &is, MyClass &object)
{
return is >> object;
}

// Version 2.0 - std::istream constructor argument, default constructors and
exceptions
class MyClass
{
Foo foo; // foo and bar require default constructors
Bar bar;
public:
MyClass(std::istream &is) // foo and bar are default initialised
{
if (!(is >> foo))
throw MyException;
if (!(is >> bar))
throw MyException;
}
};

// Version 3.0 - Shared pointers
class MyClass
{
Foo foo;
Bar bar;
public:
MyClass(Foo foo_, Bar bar_) : foo(foo_), bar(bar_) { }
};

std::istream &operator>>(std::istream &is, SharedPtr<MyClass> &object_ptr)
{
SharedPtr<Foo> foo_ptr;
if (!(is >> foo_ptr))
return is;
Foo foo = *foo_ptr;

SharedPtr<Bar> bar_ptr;
if (!(is >> bar_ptr))
return is;
Bar bar = *bar_ptr;

object_ptr = new MyClass(foo, bar);
return is;
}

// Version 4.0 - Static read methods and exceptions
class MyClass
{
Foo foo
Bar bar;
public:
MyClass(Foo foo_, Bar bar_) : foo(foo_), bar(bar_) { }

static MyClass read(std::istream &is);
};

MyClass MyClass::read(std::istream &is)
{
try {
Foo foo = Foo::read(is);
Bar bar = Bar::read(is);
return MyClass(foo, bar);
} catch (FooException e) {
throw MyException(e.get_desc());
} catch (BarException e) {
throw MyException(e.get_desc());
}
}

The third version is the only approach that does not involve default
constructors or exceptions. How else can I write my class to allow it's
objects to be read from a stream? Thanks.

When I'm writing software I find myself constantly asking, "what the heck
does A have to do with B?" In your case, what does MyClass have to do with
streams? I understand that I don't really know what MyClass is, but I'll bet
it doesn't have anything to do with streams. Thus I would be reluctant to
introduce streams into a constructor or built in function.

The obvious way is to use what I think of (non-standard term) a "delayed
constructor":

class MyClass
{
private:
Foo m_foo;
Bar m_bar;
public:
MyClass() {}
void set(const Foo &i_foo, const Bar &i_bar) {m_foo = i_foo; m_bar =
i_bar;}
...
};

The process for initialization would then be to read a temporary foo and bar
using software not shown here and then call set. Of course this might be
prohibitively expensive if Foo and Bar are large objects. (But do some run
time studies before you give up! A good optimizing compiler can sometimes
really surprise you.) If copying is really too expensive there may be
alternatives. For instance, lets say Foo is a actually a potentially large
std::vector. Then I would have a member function like this:

template <typename ITER>
set_foo(ITER first, ITER last) {std::copy(first, last,
std::back_inserter(m_foo);}

Again, no specific reference to a stream although trivially usable with a
stream using istream_iterator<>.

If all this fails I would fall back to Version 1.0. That seems like a
practical solution which mixes the concepts of MyClass and streams but at
least gets the job done efficiently.

 

[Jason Heyes]

This function does an infinite recursive call. Did you mean
object.extract(is)?

Oops. Yes I meant to write

return object.extract(is);

Why not make this function a friend of the class, and let it do what the
class extract function does, and get rid of the extract function? Why take
two steps when you can take one?

My compiler has always had problems with friend functions that overload >>.
I have gotten into the habit of writing extractors.

With this, you can't read into an existing object.

You can use the copy constructor to implement operator>> like so:

std::istream &operator>>(std::istream &is, MyClass &object)
{
object = MyClass(is);
return is;
}

You might want to catch the exceptions thrown by
MyClass::MyClass(std::istream &), writing:

std::istream &operator>>(std::istream &is, MyClass &object)
{
try {
object = MyClass(is);
} catch (MyException e) { }
return is;
}

This allows you to read into an object.

All this looks like a lot of trouble.

Of all these, version 1 (suitably modified) looks the best to me, but maybe
you have special reasons for preferring another.

I would use version 1.0 all the time if I could gaurentee that every class
had a default constructor. Here is an idea that I had:

// rename MyClass to MyClass_base
class MyClass_base
{
Foo foo;
Bar bar;
public:
MyClass_base(Foo foo_, Bar bar_) : foo(foo_), bar(bar_) { }

Foo method(Bar bar) { /* do something */ }

Foo const_method(Bar bar) const { /* do something */ }
};

// use new class MyClass that has a MyClass_base pointer
class MyClass
{
SharedPtr<MyClass_base> base;
public:
MyClass() { } // base is initialised to a null pointer

// mirror every constructor in MyClass_base
MyClass(Foo foo, Bar bar) : base(foo, bar) { }

// mirror every method in MyClass_base
Foo method(Bar bar)
{
base.make_unique(); // copy-on-write
return base->method(bar);
}

Foo const_method(Bar bar) const
{
return base->const_method(bar);
}
};

std::istream &operator>>(std::istream &is, MyClass &object)
{
Foo foo;
if (!(is >> foo))
return is;

Bar bar;
if (!(is >> bar))
return is;

object = MyClass(foo, bar);
return is;
}

If only there was a way to make this generic. It is too much work to mirror
every method of MyClass. I would rather implement a default constructor and
use version 1.0.

 

[David White]

Before going any further I think it's necessary to know exactly what you
want to do, i.e., what you have to begin with and what you want to end up
with. I hate even mentioning anything MS-related here, but a lot of what you
are saying reminds me of MFC serialization (to which the CArchive class is
central), which enables you to serialize objects of classes derived from
CObject. I believe it can also handle members that are pointers to
serialized objects, including multiple pointers to the same object.
Basically, it builds up the same object network that was previously saved.
Of course, it has its limitations (a huge one being that everything must be
derived from CObject), but if this sounds like what you are after then it's
worth looking at to get ideas. I haven't looked at MFC serialization since
MFC version 1 or 2. It might be very different now.

DW

 

[Jason Heyes]

Before going any further I think it's necessary to know exactly what you
want to do, i.e., what you have to begin with and what you want to end up
with. I hate even mentioning anything MS-related here, but a lot of what you
are saying reminds me of MFC serialization (to which the CArchive class is
central), which enables you to serialize objects of classes derived from
CObject. I believe it can also handle members that are pointers to
serialized objects, including multiple pointers to the same object.
Basically, it builds up the same object network that was previously saved.
Of course, it has its limitations (a huge one being that everything must be
derived from CObject), but if this sounds like what you are after then it's
worth looking at to get ideas. I haven't looked at MFC serialization since
MFC version 1 or 2. It might be very different now.

DW

I don't want full-fledged serialisation or MFC until I absolutely need it.
All I really want right now is to read and write objects that do not have
pointer-type members and do not necessarily have an accessible default
constructor. Let me try to give a simple example. I will use the mechanism
in version 2.0 that I wrote before (istreams and exceptions) to achieve
serialisation.

class Box
{
int left, right, up, down;

int read_int(std::istream &is)
{
int num;
if (!(is >> num))
throw ReadException;
return num;
}

public:
Box(std::istream &is) : left(read_int(is)), right(read_int(is)),
up(read_int(is)), down(read_int(is)) { }

std:stream &save(std:stream &os)
{ return os << left << right << up << down; }

int get_area() const { return (right - left) * (up - down); }
};

// program that inputs and outputs a box
// the area of the box is shown
int main()
{
Box *mybox_ptr;
try {
mybox_ptr = new Box(cin);
} catch (ReadException e) {
return 0;
}
Box mybox = *mybox_ptr;

cout << mybox << endl;
cout << mybox.get_area() << endl;
return 0;
}



The main function shows the difficulty. You can't write

Box box;
if (!(cin >> box))
return 0;

Instead you must call the istream constructor and catch ReadException. I
find this counterintuitive to C++ and annoying. Here is another design that
might be an improvement. It relies on a private default constructor, some
generic methods and a custom reference class.



template <class T>
std::istream &operator>>(std::istream &is, SharedPtr<T> &ptr)
{
T object; // default constructor must be accessible
if (!(is >> object))
return is;
ptr = new T(object);
return is;
}

template <class T>
std:stream &operator<<(std:stream &os, const SharedPtr<T> &ptr)
{
return os << *ptr;
}


class Box
{
int left, right, up, down;

// default constructor is private
Box() { }

public:
int get_area() const { return (right - left) * (up - down); }

std::istream &extract(std::istream &is)
{ return is >> left >> right >> up >> down; }

std:stream &insert(std:stream &os)
{ return os << left << right << up << down; }

// grant the extractor access to default constructor
friend std::istream &operator>>(std::istream &is, SharedPtr<Box> &box);
};

std::istream &operator>>(std::istream &is, Box &box)
{ return box.extract(is); }

std:stream &operator<<(std:stream &os, Box &box)
{ return box.insert(os); }


// box reference
class BoxRef
{
SharedPtr<Box> ptr;

public:
int get_area() const { return ptr->get_area(); }

std::istream &extract(std::istream &is) { return is >> ptr; }
std:stream &insert(std:stream &os) const { return os << ptr; }
};

std::istream &operator>>(std::istream &is, BoxRef &box)
{ return box.extract(is); }

std:stream &operator<<(std:stream &os, const BoxRef &box)
{ return box.insert(os); }


// program that inputs and outputs a box
// the area of the box is shown
int main()
{
BoxRef mybox;
if (!(cin >> mybox))
return 0;

cout << mybox << endl;
cout << mybox.get_area() << endl;
return 0;
}


Sorry for all the code. What do you think? Is this a practical solution?

 

[Cy Edmunds]

I don't want full-fledged serialisation or MFC until I absolutely need it.
All I really want right now is to read and write objects that do not have
pointer-type members and do not necessarily have an accessible default
constructor. Let me try to give a simple example. I will use the mechanism
in version 2.0 that I wrote before (istreams and exceptions) to achieve
serialisation.

class Box
{
int left, right, up, down;

int read_int(std::istream &is)
{
int num;
if (!(is >> num))
throw ReadException;
return num;
}

public:
Box(std::istream &is) : left(read_int(is)), right(read_int(is)),
up(read_int(is)), down(read_int(is)) { }

std:stream &save(std:stream &os)
{ return os << left << right << up << down; }

int get_area() const { return (right - left) * (up - down); }
};

// program that inputs and outputs a box
// the area of the box is shown
int main()
{
Box *mybox_ptr;
try {
mybox_ptr = new Box(cin);
} catch (ReadException e) {
return 0;
}
Box mybox = *mybox_ptr;

cout << mybox << endl;
cout << mybox.get_area() << endl;
return 0;
}



The main function shows the difficulty. You can't write

Box box;
if (!(cin >> box))
return 0;

Instead you must call the istream constructor and catch ReadException. I
find this counterintuitive to C++ and annoying. Here is another design that
might be an improvement. It relies on a private default constructor, some
generic methods and a custom reference class.



template <class T>
std::istream &operator>>(std::istream &is, SharedPtr<T> &ptr)
{
T object; // default constructor must be accessible
if (!(is >> object))
return is;
ptr = new T(object);
return is;
}

template <class T>
std:stream &operator<<(std:stream &os, const SharedPtr<T> &ptr)
{
return os << *ptr;
}


class Box
{
int left, right, up, down;

// default constructor is private
Box() { }

public:
int get_area() const { return (right - left) * (up - down); }

std::istream &extract(std::istream &is)
{ return is >> left >> right >> up >> down; }

std:stream &insert(std:stream &os)
{ return os << left << right << up << down; }

// grant the extractor access to default constructor
friend std::istream &operator>>(std::istream &is, SharedPtr<Box> &box);
};

std::istream &operator>>(std::istream &is, Box &box)
{ return box.extract(is); }

std:stream &operator<<(std:stream &os, Box &box)
{ return box.insert(os); }


// box reference
class BoxRef
{
SharedPtr<Box> ptr;

public:
int get_area() const { return ptr->get_area(); }

std::istream &extract(std::istream &is) { return is >> ptr; }
std:stream &insert(std:stream &os) const { return os << ptr; }
};

std::istream &operator>>(std::istream &is, BoxRef &box)
{ return box.extract(is); }

std:stream &operator<<(std:stream &os, const BoxRef &box)
{ return box.insert(os); }


// program that inputs and outputs a box
// the area of the box is shown
int main()
{
BoxRef mybox;
if (!(cin >> mybox))
return 0;

cout << mybox << endl;
cout << mybox.get_area() << endl;
return 0;
}


Sorry for all the code. What do you think? Is this a practical solution?

Jason-

This is what I meant in a previous post about not mixing up different
concepts. What does a Box have to do with streams? Nothing. Consider:

#include <iostream>
#include <fstream>

class Box
{
private:
int m_left, m_right, m_up, m_down;

public:
Box() {}

Box(int i_left, int i_right, int i_up, int i_down) :
m_left(i_left), m_right(i_right), m_up(i_up), m_down(i_down) {}

int left() const {return m_left;}

int right() const {return m_right;}

int up() const {return m_up;}

int down() const {return m_down;}
};

std::istream &
operator >> (std::istream &istr, Box &b)
{
int i_left, i_right, i_up, i_down;
istr >> i_left >> i_right >> i_up >> i_down;
b = Box(i_left, i_right, i_up, i_down);
return istr;
}

int main()
{
Box b;
std::ifstream ifs("box_numbers.txt");
ifs >> b;
if (!ifs)
{
std::cout << "oops\n";
return 1;
}
std::cout << b.left() << ' ' << b.right() << ' '
<< b.up() << ' ' << b.down() << '\n';
}

As you can see, in C++ it is easy to separate I/O streams from the objects
they work on. This is basically how std::complex works, for instance,
although in that case the class was templated and wide character streams had
to be implemented. Note that the error handling here is just the same as it
might be with any other stream operation -- the fact that it happens to be a
Box shouldn't require special treatment.

Cy

 

[Jason Heyes]

This is what I meant in a previous post about not mixing up different
concepts. What does a Box have to do with streams? Nothing. Consider:

#include <iostream>
#include <fstream>

class Box
{
private:
int m_left, m_right, m_up, m_down;

public:
Box() {}

Box(int i_left, int i_right, int i_up, int i_down) :
m_left(i_left), m_right(i_right), m_up(i_up), m_down(i_down) {}

int left() const {return m_left;}

int right() const {return m_right;}

int up() const {return m_up;}

int down() const {return m_down;}
};

std::istream &
operator >> (std::istream &istr, Box &b)
{
int i_left, i_right, i_up, i_down;
istr >> i_left >> i_right >> i_up >> i_down;
b = Box(i_left, i_right, i_up, i_down);
return istr;
}

int main()
{
Box b;
std::ifstream ifs("box_numbers.txt");
ifs >> b;
if (!ifs)
{
std::cout << "oops\n";
return 1;
}
std::cout << b.left() << ' ' << b.right() << ' '
<< b.up() << ' ' << b.down() << '\n';
}

As you can see, in C++ it is easy to separate I/O streams from the objects
they work on. This is basically how std::complex works, for instance,
although in that case the class was templated and wide character streams had
to be implemented. Note that the error handling here is just the same as it
might be with any other stream operation -- the fact that it happens to be a
Box shouldn't require special treatment.

Cy

The Box class has a default constructor that leaves it's members undefined.
Therefore the Box object created in main is not valid until

ifs >> b;

occurs. If anyone uses the Box object before that line, it will lead to
unexpected results.

I don't see the point of the other constructor. Why not write:


class Box
{
int left, right, up, down;
public:
Box() { }
friend std::istream &operator>>(std::istream &is, Box &box);
};

std::istream &operator>>(std::istream &is, Box &box)
{
return is >> box.left >> box.right >> box.up >> box.down;
}

This way you don't need the other constructor. You could also write:

class Box
{
int left, right, up, down;
public:
Box() { }

std::istream &extract(std::istream &is)
{
return is >> left >> right >> up >> down;
}
};

std::istream &operator>>(std::istream &is, Box &box)
{ return box.extract(is); }


The real problem as I see it is to get around the default constructor. You
can make the default constructor private but that still allows invalid
objects to exist. Here is what I was thinking:


class Box
{
int left, right, up, down;

Box(int left_, int right_, int up_, int down_) :
left(left_), right(right_), up(up_), down(down_)
{ }

public:
int get_area() const { return (right - left) * (up - down); }

// reads a new box
static Box *create(std::istream &is);

std::istream &extract(std::istream &is)
{
int left, right, up, down;
if (!(is >> left >> right >> up >> down))
return is; // this box is valid even when input fails
*this = Bar(left, right, up, down);
return is;
}

std:stream &insert(std:stream &os) const
{ return os << left << right << up << down; }
};

// reads a new box
Box *Box::create(std::istream &is)
{
int left, right, up, down;
if (!(is >> left >> right >> up >> down))
return 0;
return new Box(left, right, up, down);
}

// reads into an old box
std::istream &operator>>(std::istream &is, Box &box)
{ return box.extract(is); }

std:stream &operator<<(std::istream &os, const Box &box)
{ return box.insert(os); }

class BoxRef
{
SharedPtr<Box> ptr;

public:
int get_area() const { return ptr->get_area(); }

std::istream &extract(std::istream &is)
{
if (ptr) {
ptr.make_unique();
return is >> *ptr;
}

Box *new_ptr = Box::create(is);
if (!new_ptr)
return is;

ptr = new_ptr;
return is;
}

std:stream &insert(std:stream &os) const
{ return os << *ptr; }
};

std::istream &operator>>(std::istream &is, BoxRef &ref)
{ return ref.extract(is); }

std:stream &operator<<(std:stream &os, const BoxRef &ref)
{ return ref.insert(os); }

int main()
{
BoxRef mybox;
if (!(std::cin >> mybox))
return 1;
std::cout << mybox.get_area() << std::endl;
std::cout << mybox << std::endl;
return 0;
}


At no point in time does an invalid Box object exist. Instead we allow a
null reference to exist and then we read into it. What do you think?

 

[David White]

The Box class has a default constructor that leaves it's members undefined.
Therefore the Box object created in main is not valid until

ifs >> b;

occurs. If anyone uses the Box object before that line, it will lead to
unexpected results.

Well, the default constructor can put it into a valid state then.

I don't see the point of the other constructor.

It's there so you can construct a box with whatever values you want. Is that not useful?

Why not write:

class Box
{
int left, right, up, down;
public:
Box() { }
friend std::istream &operator>>(std::istream &is, Box &box);
};

std::istream &operator>>(std::istream &is, Box &box)
{
return is >> box.left >> box.right >> box.up >> box.down;
}

This way you don't need the other constructor.

Having it means that you don't even have to make the function a friend, so there is not a single
mention of std::istream cluttering up the Box class. In any case, surely such a constructor is
virtually mandatory for this class, just for general-purpose use without streams.

You could also write:

class Box
{
int left, right, up, down;
public:
Box() { }

std::istream &extract(std::istream &is)
{
return is >> left >> right >> up >> down;
}
};

std::istream &operator>>(std::istream &is, Box &box)
{ return box.extract(is); }


The real problem as I see it is to get around the default constructor. You
can make the default constructor private but that still allows invalid
objects to exist.

By 'invalid' do you mean uninitialized, or not initialized with the right values (if the default
constructor were to initialize the members to default values)? And why is an "invalid" object a
problem? You are going to make it "valid" in the very next statement after you create it. For
example, is there anything disastrous about either of these?

int value;
is >> value;

Or:

int value = 0;
is >> value;

So what if the object you are reading into isn't the right value for a moment?

Here is what I was thinking:


class Box
{
int left, right, up, down;

Box(int left_, int right_, int up_, int down_) :
left(left_), right(right_), up(up_), down(down_)
{ }

public:
int get_area() const { return (right - left) * (up - down); }

// reads a new box
static Box *create(std::istream &is);

std::istream &extract(std::istream &is)
{
int left, right, up, down;
if (!(is >> left >> right >> up >> down))
return is; // this box is valid even when input fails
*this = Bar(left, right, up, down);
return is;
}

std:stream &insert(std:stream &os) const
{ return os << left << right << up << down; }
};

// reads a new box
Box *Box::create(std::istream &is)
{
int left, right, up, down;
if (!(is >> left >> right >> up >> down))
return 0;
return new Box(left, right, up, down);
}

// reads into an old box
std::istream &operator>>(std::istream &is, Box &box)
{ return box.extract(is); }

As I said in my first post (if I remember correctly back that far), why not ditch the extract
function in the class and make this function a friend so it can set the members directly? That
way you don't clutter your Box class with stream stuff.

std:stream &operator<<(std::istream &os, const Box &box)
{ return box.insert(os); }

class BoxRef
{
SharedPtr<Box> ptr;

public:
int get_area() const { return ptr->get_area(); }

std::istream &extract(std::istream &is)
{
if (ptr) {
ptr.make_unique();
return is >> *ptr;
}

Box *new_ptr = Box::create(is);
if (!new_ptr)
return is;

ptr = new_ptr;
return is;
}

std:stream &insert(std:stream &os) const
{ return os << *ptr; }
};

std::istream &operator>>(std::istream &is, BoxRef &ref)
{ return ref.extract(is); }

std:stream &operator<<(std:stream &os, const BoxRef &ref)
{ return ref.insert(os); }

int main()
{
BoxRef mybox;
if (!(std::cin >> mybox))
return 1;
std::cout << mybox.get_area() << std::endl;
std::cout << mybox << std::endl;
return 0;
}


At no point in time does an invalid Box object exist. Instead we allow a
null reference to exist and then we read into it. What do you think?

I think it's extremely and unnecessarily complicated. Apart from the new Box option (which could
easily be added to Cy's version), what are the advantages of your version?

DW

 

[Cy Edmunds]

[snip]

I don't see the point of the other constructor. Why not write:


class Box
{
int left, right, up, down;
public:
Box() { }
friend std::istream &operator>>(std::istream &is, Box &box);
};

std::istream &operator>>(std::istream &is, Box &box)
{
return is >> box.left >> box.right >> box.up >> box.down;
}

The meaning of Box is very tightly coupled to its implementation. There is
nothing wrong with that -- I could say the same thing about several standard
library classes such as std::complex. It's just a simple concrete data type.
Such a data type which lacks a constructor which directly initializes the
data members is really strange. Your design says that a Box isn't really a
Box unless it is initialized from a stream!

In general, it is a poor idea to break encapsulation by implementing any
convenience function, including operator >>, using a member function. If you
only use the interface to the object the software will be more maintainable.

 

[Jason Heyes]

[snip]

I don't see the point of the other constructor. Why not write:


class Box
{
int left, right, up, down;
public:
Box() { }
friend std::istream &operator>>(std::istream &is, Box &box);
};

std::istream &operator>>(std::istream &is, Box &box)
{
return is >> box.left >> box.right >> box.up >> box.down;
}

The meaning of Box is very tightly coupled to its implementation. There is
nothing wrong with that -- I could say the same thing about several standard
library classes such as std::complex. It's just a simple concrete data type.
Such a data type which lacks a constructor which directly initializes the
data members is really strange. Your design says that a Box isn't really a
Box unless it is initialized from a stream!

In general, it is a poor idea to break encapsulation by implementing any
convenience function, including operator >>, using a member function. If you
only use the interface to the object the software will be more maintainable.

Let me write another Box class. It might help spell out the difficulty that
I have with object creation and streaming I/O. This time I have added a
class invariant. The invariant must always hold true. If it doesn't hold
true then the Box is invalid. There are a few strange ideas in this code so
bear with me.

#include <iostream>
#include <string>
#include <stdexcept>

template <class T>
class Phantom
{
T *t;
public:
Phantom() : t(0) { }
Phantom(T *t_) : t(t_) { }
~Phantom() { delete t; }

Phantom &operator=(const Phantom &rhs)
{
if (this != &rhs) {
delete t;
t = rhs.t;
}
return *this;
}

const T &copy() const
{
if (!t)
throw std::runtime_error("Cannot copy from an empty phantom
object");
return T(*t);
}
};

class Box
{
int left, right, up, down;
// invariant : left < right && down < up

Box(int l, int r, int u, int d) : left(l), right(r), up(u), down(d) { }

public:
int get_area() const { return (right - left) * (up - down); }

static std::istream &read(std::istream &is, int &l, int &r, int &u, int
&d);
static bool validate(int l, int r, int u, int d);

friend std::istream &operator>>(std::istream &is, Phantom<Box> &ptm);
friend std::istream &operator>>(std::istream &is, Box &box);
};

std::istream &Box::read(std::istream &is, int &l, int &r, int &u, int &d)
{
return is >> l >> r >> u >> d;
}

bool Box::validate(int l, int r, int u, int d)
{
return l < r && u < d;
}

std::istream &operator>>(std::istream &is, Phantom<Box> &ptm)
{
int l, r, u, d;
if (read(is, l, r, u, d) && validate(l, r, u, d))
ptm = new Box(l, r, u, d);
else // !is || !validate(l, r, u, d)
is.clear(ios::failbit);
return is;
}

std::istream &operator>>(std::istream &is, Box &box)
{
int l, r, u, d;
if (read(is, l, r, u, d) && validate(l, r, u, d))
box = Box(l, r, u, d);
else // !is || !validate(l, r, u, d)
is.clear(ios::failbit);
return is;
}

using namespace std;

// extracts two Box objects from std::cin
// area is calculated for each
int main()
{
Phantom<Box> ptm;
if (!(cin >> ptm))
return 1;
Box mybox = ptm.copy();
cout << mybox.get_area() << endl;

if (!(cin >> mybox))
return 1;
cout << mybox.get_area() << endl;
return 0;
}

There is a good reason why I did not make the Box constructor public. If
that constructor were made public, it would allow invalid Box objects to be
created outside. The constructor is private and directly initialises the
data members of Box.

Obviously it would be a mistake to add a default constructor that
initialises the data members to default values. This is why I have taken
pains to create the a generic Phantom class. What do you think of this?

 

[David White]

class Box
{
int left, right, up, down;
// invariant : left < right && down < up

Box(int l, int r, int u, int d) : left(l), right(r), up(u), down(d) { }

public:
int get_area() const { return (right - left) * (up - down); }

static std::istream &read(std::istream &is, int &l, int &r, int &u, int
&d);
static bool validate(int l, int r, int u, int d);

friend std::istream &operator>>(std::istream &is, Phantom<Box> &ptm);
friend std::istream &operator>>(std::istream &is, Box &box);
};

std::istream &Box::read(std::istream &is, int &l, int &r, int &u, int &d)
{
return is >> l >> r >> u >> d;
}

bool Box::validate(int l, int r, int u, int d)
{
return l < r && u < d;
}

std::istream &operator>>(std::istream &is, Phantom<Box> &ptm)
{
int l, r, u, d;
if (read(is, l, r, u, d) && validate(l, r, u, d))
ptm = new Box(l, r, u, d);
else // !is || !validate(l, r, u, d)
is.clear(ios::failbit);
return is;
}

std::istream &operator>>(std::istream &is, Box &box)
{
int l, r, u, d;
if (read(is, l, r, u, d) && validate(l, r, u, d))
box = Box(l, r, u, d);
else // !is || !validate(l, r, u, d)
is.clear(ios::failbit);
return is;
}

using namespace std;

// extracts two Box objects from std::cin
// area is calculated for each
int main()
{
Phantom<Box> ptm;
if (!(cin >> ptm))
return 1;
Box mybox = ptm.copy();
cout << mybox.get_area() << endl;

if (!(cin >> mybox))
return 1;
cout << mybox.get_area() << endl;
return 0;
}

There is a good reason why I did not make the Box constructor public. If
that constructor were made public, it would allow invalid Box objects to be
created outside.

You can throw an exception if that happens. How can anyone use the Box class if you don't allow
one to be constructed?

The constructor is private and directly initialises the
data members of Box.

Obviously it would be a mistake to add a default constructor that
initialises the data members to default values.

Why? The default constructor can create a Box that's valid. And then the >> function can check
the validity of the left, right etc. values read from the stream and fail if they aren't right.

I think I must not be getting something here. How is your Box class different from any
equivalent Rectangle class that might represent a window or some other useful concept, and could
be used all over the place in some applications?

DW

 

[Cy Edmunds]

[snip]

I don't see the point of the other constructor. Why not write:


class Box
{
int left, right, up, down;
public:
Box() { }
friend std::istream &operator>>(std::istream &is, Box &box);
};

std::istream &operator>>(std::istream &is, Box &box)
{
return is >> box.left >> box.right >> box.up >> box.down;
}

The meaning of Box is very tightly coupled to its implementation. There is
nothing wrong with that -- I could say the same thing about several standard
library classes such as std::complex. It's just a simple concrete data type.
Such a data type which lacks a constructor which directly initializes the
data members is really strange. Your design says that a Box isn't really a
Box unless it is initialized from a stream!

In general, it is a poor idea to break encapsulation by implementing any
convenience function, including operator >>, using a member function. If you
only use the interface to the object the software will be more maintainable.

Let me write another Box class. It might help spell out the difficulty that
I have with object creation and streaming I/O. This time I have added a
class invariant. The invariant must always hold true. If it doesn't hold
true then the Box is invalid. There are a few strange ideas in this code so
bear with me.

#include <iostream>
#include <string>
#include <stdexcept>

template <class T>
class Phantom
{
T *t;
public:
Phantom() : t(0) { }
Phantom(T *t_) : t(t_) { }
~Phantom() { delete t; }

Phantom &operator=(const Phantom &rhs)
{
if (this != &rhs) {
delete t;
t = rhs.t;
}
return *this;
}

const T &copy() const
{
if (!t)
throw std::runtime_error("Cannot copy from an empty phantom
object");
return T(*t);
}
};

class Box
{
int left, right, up, down;
// invariant : left < right && down < up

Box(int l, int r, int u, int d) : left(l), right(r), up(u), down(d) { }

public:
int get_area() const { return (right - left) * (up - down); }

static std::istream &read(std::istream &is, int &l, int &r, int &u, int
&d);
static bool validate(int l, int r, int u, int d);

friend std::istream &operator>>(std::istream &is, Phantom<Box> &ptm);
friend std::istream &operator>>(std::istream &is, Box &box);
};

std::istream &Box::read(std::istream &is, int &l, int &r, int &u, int &d)
{
return is >> l >> r >> u >> d;
}

bool Box::validate(int l, int r, int u, int d)
{
return l < r && u < d;
}

std::istream &operator>>(std::istream &is, Phantom<Box> &ptm)
{
int l, r, u, d;
if (read(is, l, r, u, d) && validate(l, r, u, d))
ptm = new Box(l, r, u, d);
else // !is || !validate(l, r, u, d)
is.clear(ios::failbit);
return is;
}

std::istream &operator>>(std::istream &is, Box &box)
{
int l, r, u, d;
if (read(is, l, r, u, d) && validate(l, r, u, d))
box = Box(l, r, u, d);
else // !is || !validate(l, r, u, d)
is.clear(ios::failbit);
return is;
}

using namespace std;

// extracts two Box objects from std::cin
// area is calculated for each
int main()
{
Phantom<Box> ptm;
if (!(cin >> ptm))
return 1;
Box mybox = ptm.copy();
cout << mybox.get_area() << endl;

if (!(cin >> mybox))
return 1;
cout << mybox.get_area() << endl;
return 0;
}

There is a good reason why I did not make the Box constructor public. If
that constructor were made public, it would allow invalid Box objects to be
created outside. The constructor is private and directly initialises the
data members of Box.

Obviously it would be a mistake to add a default constructor that
initialises the data members to default values. This is why I have taken
pains to create the a generic Phantom class. What do you think of this?

I think you have made a simple class virtually useless. For instance, take
this design and try to make an array of them:

Box disaster[3]; // error
std::vector<Box> wontworkeither; // error

Try to write a function which extends a Box's functionality:

// scale parameters by a constant
Box &operator *= (int factor)
{
// can't implement without using streams!
}

Evidently my argument that an unitialized Box is no worse than an
uninitialized int didn't convince you. But your idea that a default
initialized Box is some sort of weapon of mass destruction is far from the
mainstream of C++ design practice. And having a simple concrete data type
which cannot be initialized from arguments makes it very difficult to
extend.

You have obviously given the matter considerable thought and I think your
solution with the Phantom class is quite clever. However, I would say its
objective is unsound.

 

[Jason Heyes]

You can throw an exception if that happens.

Exceptions are annoying. I like streams better.

How can anyone use the Box class if you don't allow one to be constructed?

The main function shows how to use the Box class. If you want the Box class
to do other things, then you had better extend it. For the purposes of this
discussion, however, that is not necessary.

Why? The default constructor can create a Box that's valid. And then the
the validity of the left, right etc. values read from the stream and fail if they aren't right.

Let me ask you something. Should every class have a default constructor?
Should the Box class have a default constructor? You say the default
constructor can create a Box that is valid. But how do you decide on which
Box to create? What I am saying is that sometimes it doesn't make sense to
have a default constructor. Do you agree?

I think I must not be getting something here. How is your Box class different from any
equivalent Rectangle class that might represent a window or some other useful concept, and could
be used all over the place in some applications?

DW

I don't know what Rectangle class you are referring to. The Box class is
very simple. You can read and write Boxes using streams and get the area of
a Box. You cannot create a Box without reading it from a stream. Is that so
wrong? If you want you can modify the class to have this:

Box::Box(int l, int r, int u, int d) : left(l), right(r), up(u), down(d)
{
if (!validate(l, r, u, d))
throw std::runtime_error("Invalid arguments");
}

It still doesn't solve my initial problem. How will you read your first Box
object from a stream?

 

[Jason Heyes]

I think you have made a simple class virtually useless. For instance, take
this design and try to make an array of them:

Box disaster[3]; // error

Arrays are evil.

std::vector<Box> wontworkeither; // error

No. That will work. Look at this function.

std::istream &read_boxes(std::istream &is, std::vector<Box> &boxes, int
count)
{
if (count == 0)
return is;

Phantom<Box> phm; // read first Box
if (!(is >> phm))
return is;
Box box = phm.copy();
boxes.push_back(box);

// read the rest
for (int i=1; i < count; i++) {
if (!(is >> box))
return is;
boxes.push_back(box);
}

return is;
}

The class is not useless.

Try to write a function which extends a Box's functionality:

// scale parameters by a constant
Box &operator *= (int factor)
{
// can't implement without using streams!
}

What do you mean? Here is an implementation:

Box &Box:perator*=(int factor)
{
left *= factor; right *= factor;
up *= factor; down *= factor;
}

What is the problem?

Evidently my argument that an unitialized Box is no worse than an
uninitialized int didn't convince you. But your idea that a default
initialized Box is some sort of weapon of mass destruction is far from the
mainstream of C++ design practice. And having a simple concrete data type
which cannot be initialized from arguments makes it very difficult to
extend.

All I am saying is that sometimes it doesn't make sense to have a default
constructor. The Box is just a simple example of this. There is nothing
wrong with an "uninitialised" (initialised but not explicitly initialised)
int. It is an int. That makes sense. A Box on the other hand is a more
complicated concept. A Box must be read from a stream. It cannot be created
by any other means.

You have obviously given the matter considerable thought and I think your
solution with the Phantom class is quite clever. However, I would say its
objective is unsound.

Well thanks for the compliment. And I appreciate the time that you have
devoted to this issue.

 

[Cy Edmunds]

I think you have made a simple class virtually useless. For instance, take
this design and try to make an array of them:

Box disaster[3]; // error

Arrays are evil.

std::vector<Box> wontworkeither; // error

No. That will work. Look at this function.

std::istream &read_boxes(std::istream &is, std::vector<Box> &boxes, int
count)

[snip]

This requires a reference to a std::vector which you can't declare because
Box is not default constructable.

What do you mean? Here is an implementation:

Box &Box:perator*=(int factor)
{
left *= factor; right *= factor;
up *= factor; down *= factor;
}

What is the problem?

Oops, I goofed! I meant to write

// scale parameters by a constant
Box &operator *= (Box &b, int factor)
{
// can't implement without using streams!
}

In other words, NOT a member function. Since that is the distinction I
wished to make, that was a serious error!

[snip]

A Box must be read from a stream. It cannot be created
by any other means.

That's why I don't like it.

 

[David White]

Exceptions are annoying. I like streams better.

They aren't interchangeable.

The main function shows how to use the Box class. If you want the Box class
to do other things, then you had better extend it. For the purposes of this
discussion, however, that is not necessary.
fail
if they aren't right.

Let me ask you something. Should every class have a default constructor?
No.

Should the Box class have a default constructor?

I wouldn't find it useful without one.

You say the default
constructor can create a Box that is valid. But how do you decide on which
Box to create?

Whatever you like. A "unit Box" - 0,1,1,0 - is the obvious default, just as
zero is the obvious default for type int (and is what you get is you write
int()).

What I am saying is that sometimes it doesn't make sense to
have a default constructor. Do you agree?

Yes, but not in this case. If you have a default constructor the stream code
is quite simple. Without one it's unnecessarily complicated.

Not having a default constructor can be a real pain sometimes. If you have a
Box as a member of another class and there is no default constructor, the
class is forced to initialize it somehow in its own constructor, even if it
won't know what values it's supposed to have until later. In general, if
there's a reasonable default, I'd have a default constructor. It just makes
life easier.

I don't know what Rectangle class you are referring to.

For example, for a graphics library of mine I wrote two rectangle classes.
One is LogRect, or "logical rectangle" which represents the logical, or
integer, units on the graphics device on which drawing takes place. The
other is VirRect, or "virtual rectangle", which represents a floating-point
scale of the user's choice. All drawable objects are defined in virtual
units, which are later converted to logical units for drawing. Within the
library source code, the type 'LogRect' appears 623 times, and the type
'VirRect' appears 902 times. These objects are used everywhere. Using them
would have been extraordinarily difficult if they didn't have default
constructors. To give just one example, the base class of all drawable
objects has two VirRects and one LogRect. It is not known what values some
or all of these rectangles will have until after the drawable object is
created.

The Box class is very simple.

It's the simple classes that tend to be used the most, and for which a
default constructor is most useful.

You can read and write Boxes using streams and get the area of
a Box. You cannot create a Box without reading it from a stream. Is that so
wrong?

If that's what suits your purpose, no, but it's not something I can imagine
ever wanting.

If you want you can modify the class to have this:

Box::Box(int l, int r, int u, int d) : left(l), right(r), up(u), down(d)
{
if (!validate(l, r, u, d))
throw std::runtime_error("Invalid arguments");
}

It still doesn't solve my initial problem. How will you read your first Box
object from a stream?

Box b;
if(is >> b)
{
// whatever
}

DW

 

[Jason Heyes]

They aren't interchangeable.

I know.

I wouldn't find it useful without one.

What do you mean "useful"?

Whatever you like. A "unit Box" - 0,1,1,0 - is the obvious default, just as
zero is the obvious default for type int (and is what you get is you write
int()).

What about the box centered at the origin -1,1,1,-1? Isn't that an obvious
default to use?

Yes, but not in this case. If you have a default constructor the stream code
is quite simple. Without one it's unnecessarily complicated.

The stream code is almost the same. The main difference is that you need

std::istream &operator>>(std::istream &, Box *&);

to read the first Box.

Not having a default constructor can be a real pain sometimes. If you have a
Box as a member of another class and there is no default constructor, the
class is forced to initialize it somehow in its own constructor, even if it
won't know what values it's supposed to have until later. In general, if
there's a reasonable default, I'd have a default constructor. It just makes
life easier.

And if there isn't a reasonable default, you start to think that there is
something wrong with your class. What's worse is when you have a default
that doesn't make any sense, and your are using it throughout your program.

For example, for a graphics library of mine I wrote two rectangle classes.
One is LogRect, or "logical rectangle" which represents the logical, or
integer, units on the graphics device on which drawing takes place. The
other is VirRect, or "virtual rectangle", which represents a floating-point
scale of the user's choice. All drawable objects are defined in virtual
units, which are later converted to logical units for drawing. Within the
library source code, the type 'LogRect' appears 623 times, and the type
'VirRect' appears 902 times. These objects are used everywhere. Using them
would have been extraordinarily difficult if they didn't have default
constructors. To give just one example, the base class of all drawable
objects has two VirRects and one LogRect. It is not known what values some
or all of these rectangles will have until after the drawable object is
created.

If you declare objects as you use them, you won't have a problem. For
instance, rather than write

Box box;
// code fragment
box = whatever;

you can write

// code fragment
Box box = whatever;

This way you don't abuse the default constructor.

It's the simple classes that tend to be used the most, and for which a
default constructor is most useful.

Even for simple classes such as Box, however, a default constructor is a bad
idea.

If that's what suits your purpose, no, but it's not something I can imagine
ever wanting.

It doesn't have to be Boxes that you read and write. It could be an image
file like a TGA, GIF, BMP, etc.

Box b;
if(is >> b)
{
// whatever
}

DW

You create a default Box and then write straight over it. The default Box
was never used. Therefore it should never have been created. Here is a
correct version that does not use the default constructor:

int l, r, u, d;
if (is >> l >> r >> u >> d)
{
Box b(l, r, u, d);
// whatever
}

 

[Jason Heyes]

std::istream &read_boxes(std::istream &is, std::vector<Box> &boxes, int
count)

[snip]

This requires a reference to a std::vector which you can't declare because
Box is not default constructable.

You can declare an empty std::vector, regardless of whether Box is default
constructable or not.

Oops, I goofed! I meant to write

// scale parameters by a constant
Box &operator *= (Box &b, int factor)
{
// can't implement without using streams!
}

In other words, NOT a member function. Since that is the distinction I
wished to make, that was a serious error!

Make it a friend of Box and everything works fine.

That's why I don't like it.

What difference does it make? The input can come from constructor arguments
or it can come from a stream. What is the difference?