throwing exception from constructor

[Brian Gideon]

My approach is to do what's appropriate in the constructor to create a
viable object. In the case of something like FileStream, that involves
opening the file - which would naturally involve throwing an exception
if the file cannot be opened.

The FileStream constructor is another example of a poor design choice.
IMHO, opening a file is not a simple operation. There should be an
Open method (either instance or static) that performs this work. Even
after reading the documentation it's still not perfectly clear that the
constructor actually opens the file. But the deed has been done and
throwing exceptions is the only appropriate choice.

Brian

 

[Barry Kelly]

"Carl Daniel [VC++ MVP]"

I'd really like to hear those "difficulties". It's quite normal to throw
exceptions from C++ constructors - it's the one of the main reasons that
exceptions were added to C++.

I know - I mentioned it in a parallel post.

Consider (implementions inlined, copy constructors / assignment
operators / auto_ptr etc. all ignored, for the sake of exposition):

---8<---
class Foo
{
private:
Bar *m_bar;

public:
Foo(bool doThrow)
{
m_bar = new Bar;
if (doThrow)
throw 0;
}

~Foo()
{
delete m_bar;
}
};
--->8---

The same in Delphi:

---8<---
type
Foo = class
private
FBar: Bar;

public
constructor Create(doThrow: Boolean);
// inlined here for exposition, not actually valid Delphi syntax
begin
FBar = Bar.Create;
if doThrow then
throw Exception.Create;
end;

destructor Destroy; override;
begin
FBar.Free; // Free() checks for nil pointer first
end;

end;
--->8---

-- Barry

 

[Jon Skeet [C# MVP]]

The FileStream constructor is another example of a poor design choice.
IMHO, opening a file is not a simple operation. There should be an
Open method (either instance or static) that performs this work. Even
after reading the documentation it's still not perfectly clear that the
constructor actually opens the file. But the deed has been done and
throwing exceptions is the only appropriate choice.

Having a FileStream which hasn't opened the file seems pretty odd to me
- and would mean that you'd have extra code in everything which used
it. Having no public constructors, just static methods, would be doable
- but I don't see the advantage. From the client's point of view
there'd still be an exception occurring.

Where's the benefit in making the constructor *not* open the file? I'm
all for constructors only doing a sensible amount of work - but not
when that means that an object isn't actually ready for use until
another particular method has been called.

 

[Barry Kelly]

The FileStream constructor is another example of a poor design choice.

I strongly disagree!

IMHO, opening a file is not a simple operation. There should be an
Open method (either instance or static) that performs this work.

This would create a three-state stream - closed, open and disposed -
rather than what we currently have, two states - open and disposed. With
an 'Open' operation, one would expect to be able to close and open it
repeatedly. I, for one, prefer objects with simple purposes in life and
with a minimum of 'modality'.

-- Barry

 

[Brian Gideon]

I strongly disagree!

Well, it's definitely debatable. I can't honestly say that I even
completely agree with my own statement nor do I always follow the
"simple operation" rule for constructors myself. I just think there is
a better alternative in this case.

This would create a three-state stream - closed, open and disposed -
rather than what we currently have, two states - open and disposed. With
an 'Open' operation, one would expect to be able to close and open it
repeatedly. I, for one, prefer objects with simple purposes in life and
with a minimum of 'modality'.

And you'd get just that with a static factory method; a method with a
verb name clearly describing what it is going to do.

 

[Jeff Louie]

Sek.. You can throw an exception in the constructor.. or use a static
class factory
and return null on failure.. or set a flag isValid to false.

Beware that the finalizer will be called in C# even on exceptional
construction!

http://www.geocities.com/Jeff_Louie/oop30.htm

C++ and PHP coders will be surprised to learn that in C# even if an
exception is
thrown in the user defined constructor, the user defined destructor will
still be
called.

Regards,
Jeff
Thats the only way i could think of to denote the failure of
constructor, wherein i am invoking couple of other classes to
initialise the object.<<

 

[Barry Kelly]

Well, it's definitely debatable. I can't honestly say that I even
completely agree with my own statement nor do I always follow the
"simple operation" rule for constructors myself. I just think there is
a better alternative in this case.


And you'd get just that with a static factory method; a method with a
verb name clearly describing what it is going to do.

One already has 'File.Open' and related methods which provide the
appropriate arguments to the FileStream constructor.

But on the other hand - what's the benefit from using this approach? If
one calls the static method rather than using the constructor, what's
been gained over the losses: extra complexity, the divorce between where
the object is constructed (the static method) and the actual object
implementation, and the extra mode on the file stream? Also, won't all
streams now need this Open() method? What do you do when you get a
stream from elsewhere, like a NetworkStream from a socket? Do you need
to open that too? If not, where does all this documentation go?

-- Barry

 

[Barry Kelly]

Sek.. You can throw an exception in the constructor.. or use a static
class factory
and return null on failure.. or set a flag isValid to false.

Beware that the finalizer will be called in C# even on exceptional
construction!

http://www.geocities.com/Jeff_Louie/oop30.htm

C++ and PHP coders will be surprised to learn that in C# even if an
exception is
thrown in the user defined constructor, the user defined destructor will
still be
called.

On the other hand, Delphi programmers won't be surprised - although
calling C# '~Class()' syntax a 'destructor' is, of course, an abuse of
terminology...

-- Barry

 

[Brian Gideon]

One already has 'File.Open' and related methods which provide the
appropriate arguments to the FileStream constructor.

But on the other hand - what's the benefit from using this approach?

File.Open doesn't have an ambiguous meaning. It's perfectly clear from
the name that a file will be opened if no exceptions are thrown. The
problem is that it's on a separate class, which is fine, but how would
one know to look at the File class if they're currently interested in
the FileStream class. It makes more sense to add it to the FileStream
class as well. Consider the following lines of code.

FileStream stream = new FileStream("foo.txt");
FileStream stream = FileStream.Open("foo.txt");

Without knowing anything about a FileStream what can we say about these
lines of code? The only thing we can say for certain about the first
line is that stream can be used to read/write data in the foo.txt file.
We can say more about the second. Not only can it read/write to
foo.txt, but it is *ready* to do so since the method clearly indicates
that the stream is open when it's returned.

But, that's not why I think it was a poor design choice. I think it's
a poor design choice because a constructor that throws an IOException
has a complex operation embedded in it. That contradicts the following
guideline.

"Do minimal work in the constructor. Constructors should not do much
work other than to capture the constructor parameters. The cost of any
other processing should be delayed until required." --Framework Design
Guidelines by Cwalina & Abrams.

If
one calls the static method rather than using the constructor, what's
been gained over the losses: extra complexity, the divorce between where
the object is constructed (the static method) and the actual object
implementation, and the extra mode on the file stream?

I don't see where there's extra complexity. In fact, from the callers
perspective the interface would be more self describing and thus
simpler to use!

You wouldn't necessarily have to move logic around to accomodate the
factory method. Just make the constructor private and have the factory
call that if you like.

Using the static factory method approach it would still be impossible
to reopen a closed stream since there wouldn't be an instance method
there to do it.

Also, won't all
streams now need this Open() method?

Yep. That's how most of the objects that acquire resources in the
System.Net and System.Data namespaces work. Though, in those examples
they're usually instance methods.

What do you do when you get a
stream from elsewhere, like a NetworkStream from a socket? Do you need
to open that too? If not, where does all this documentation go?

I'd say no especially if the documentation on the stream indicated that
a caller can assume it's open if the Close method hasn't been
previously called. That's pretty much how it works now. The same
semantics can be acheived with a static factory method.

 

[Barry Kelly]

"Do minimal work in the constructor. Constructors should not do much
work other than to capture the constructor parameters. The cost of any
other processing should be delayed until required." --Framework Design
Guidelines by Cwalina & Abrams.

I agree with the general principle that constructors should not initiate
an operation that is costly in time or space, but I don't agree with the
notion that a constructor should delay 'full initialization with all
invariants holding' until a later 'Initialize' method is called. All
that does is complicate the interface, multiply the possible states,
etc. etc. I'm more interested in how you think the rationale behind the
guideline applies in this particular case.

Or to put it another way, I think there's a balance to be achieved -
described below.

I don't see where there's extra complexity. In fact, from the callers
perspective the interface would be more self describing and thus
simpler to use!

I can only talk about my personal experience using different classes. I
personally find FileStream nice and easy to work with, and I like its
idiom. On the other hand, I find System.Diagnostics.Process far more
awkward. I need to construct an instance, fiddle with various properties
before finally invoking a method.

Now, starting a process etc. is an expensive operation. As such, it
shouldn't occur inside the constructor. What's more, the various
subtleties and options on how one can create a process aren't fully
appreciated by people who are new to the functionality. With respect to
creating a process, I think the static method approach along with a
do-very-little constructor and tweakable properties works well - though
the static methods are missing in the current version of the BCL.

Thus, I think it's a question of where one draws the line. I think that
FileStream is good and that making it a tweakable stillborn object until
you finally call 'Open()' would be overkill. Just MHO.

I think it has more to do with the cost of the operation than the fact
that it may or may not throw exceptions.

You wouldn't necessarily have to move logic around to accomodate the
factory method. Just make the constructor private and have the factory
call that if you like.

Using the static factory method approach it would still be impossible
to reopen a closed stream since there wouldn't be an instance method
there to do it.

This is more an argument for named constructors, but I think that's an
orthogonal topic to throwing exceptions from constructors.

Yep. That's how most of the objects that acquire resources in the
System.Net and System.Data namespaces work. Though, in those examples
they're usually instance methods.

But these objects typically block for lengthy time periods. I would
agree to a rule of thumb that says "any operation that you might
consider making asynchronous doesn't belong in a constructor".

-- Barry

 

[Barry Kelly]

FBar = Bar.Create;
if doThrow then
throw Exception.Create;

I've been programming in too many different languages lately. That
should be:

FBar := Bar.Create;
if doThrow then
raise Exception.Create('blah');

-- Barry

 

[Carl Daniel [VC++ MVP]]

I think I must have heard exaggerated claims of the problems involved.
I believe (after a quick search) that after an exception is thrown in
a
C++ constructor that the destructor is not called, so resources need
to
be cleaned up in that situation. I guess some people have taken that
as
a reason not to throw exceptions at all in C++...

That's correct - the body of the destructor is not run, but the destructors
of all base classes and member variables are run (assuming the exception was
thrown from the body of the constructor). If you use the RAII pattern (as
all modern C++ programmers should), your constructor and destructor bodies
will almost always be empty and throwing an exception from the constructor
will do exactly what you'd want it to do.

-cd

 

[adebaene]

Barry Kelly wrote:

Consider (implementions inlined, copy constructors / assignment
operators / auto_ptr etc. all ignored, for the sake of exposition):

---8<---
class Foo
{
private:
Bar *m_bar;

public:
Foo(bool doThrow)
{
m_bar = new Bar;
if (doThrow)
throw 0;
}

~Foo()
{
delete m_bar;
}
};
--->8---

I think you're projecting your C# way of thinking onto C++, therefore
doing unwise design choices...

Why is the m_bar member allocated allocated on the heap? Any reason for
that in the 1st place ???? 9 times out of 10, the good solution is as
imple as :

class Foo
{
Bar m_bar; //no need for constructor nor destructor
};



Now, suppose there is a good reason to allocate m_bar on the heap, what
about doing modern C++, eg usig the RAII idiom ??

class Foo
{
std::auto_ptr<Bar> m_bar; //depending on context,
boost::shared_ptr may be another option

public:
Foo(bool doThrow)
: m_bar(new Bar())
{
if (doThrow)
throw 0;
}
};

Arnaud
MVP - VC

 

[Brian Gideon]

Having a FileStream which hasn't opened the file seems pretty odd to me
- and would mean that you'd have extra code in everything which used
it. Having no public constructors, just static methods, would be doable
- but I don't see the advantage. From the client's point of view
there'd still be an exception occurring.

Where's the benefit in making the constructor *not* open the file? I'm
all for constructors only doing a sensible amount of work - but not
when that means that an object isn't actually ready for use until
another particular method has been called.

I realize I'm splitting hairs on this one, but the main advantage IMHO
is better compliance with the guideline. Anything that throws an
IOException can't possibly be a simple operation. If you use the
static factory method approach then you would have an object that's
ready to use in one call with the added benefit that the interface
should be easier to understand for the caller.

I think one advantage of the guideline is that it encourages developers
to put complex operations in methods where 1) they're expected and 2)
that have self describing names. To me anyway, line 2 is an order of
magnitude more clear that the stream is open than line 1.

FileStream stream = new FileStream("foo.txt"); // Line 1
FileStream stream = FileStream.Open("foo.txt"); // Line 2

I'm not saying that throwing exceptions from a constructor is bad. In
fact the same guidelines say it's acceptable when appropriate. Since
the FileStream does attempt to open a file then throwing an exception
is a logical choice.

 

[Barry Kelly]

Barry Kelly wrote:


I think you're projecting your C# way of thinking onto C++, therefore
doing unwise design choices...

Why is the m_bar member allocated allocated on the heap?

That's irrelevant. This isn't a program, so there are no design choices.
There are only language features.

Now, suppose there is a good reason to allocate m_bar on the heap, what
about doing modern C++, eg usig the RAII idiom ??

I explicitly mentioned that I was ignoring auto_ptr for the sake of
exposition.

In my view, C++'s destructors along with the RAII pattern are basically
required to be overused in order to hack around a limitation in the
language. The RAII pattern works very well for simple scenarios (and
they should be used in such reusable scenarios), like an owned pointer
or a ref-counted pointer, but not every paired operation between
constructor and destructor is so reusable that it merits a new type.

-- Barry

 

[Jon Skeet [C# MVP]]

I realize I'm splitting hairs on this one, but the main advantage IMHO
is better compliance with the guideline. Anything that throws an
IOException can't possibly be a simple operation. If you use the
static factory method approach then you would have an object that's
ready to use in one call with the added benefit that the interface
should be easier to understand for the caller.

To split hairs even further, I regard compliance with guidelines per se
as not being an advantage at all - it's only an advantage to the extent
that the guideline itself makes sense. Clearly I disagree with the
generality of the guideline in this case, so compliance with it is
irrelevant to me.

I think one advantage of the guideline is that it encourages developers
to put complex operations in methods where 1) they're expected and 2)
that have self describing names. To me anyway, line 2 is an order of
magnitude more clear that the stream is open than line 1.

*That* is much more of an argument I can at least understand. I still
disagree with it, largely because opening the stream on construction is
precisely the behaviour I expect, but at least it's a reasonable
argument

 

[Arnaud Debaene]

(e-mail address removed) wrote:

In my view, C++'s destructors along with the RAII pattern are basically
required to be overused in order to hack around a limitation in the
language.

Which limitation? The limitation is rather on the .NET finalizer side IMHO :
since you don't know when they are run, there is almost nothing you can do
inside them!

The RAII pattern works very well for simple scenarios (and
they should be used in such reusable scenarios), like an owned pointer
or a ref-counted pointer, but not every paired operation between
constructor and destructor is so reusable that it merits a new type.

That's why there are generic solutions to the RAII idiom, such as ScopeGuard
(http://www.ddj.com/dept/cpp/184403758)

Arnaud
MVP - VC

 

[Barry Kelly]

Which limitation? The limitation is rather on the .NET finalizer side IMHO :
since you don't know when they are run, there is almost nothing you can do
inside them!

We'll have to agree to disagree - I think it's a limitation of the
language definition, nothing to do with .NET or otherwise, in the
context of "throwing an exception from the constructor".

That's why there are generic solutions to the RAII idiom, such as ScopeGuard
(http://www.ddj.com/dept/cpp/184403758)

Yes, more hacks and crutches...

-- Barry

 

[Arnaud Debaene]

We'll have to agree to disagree - I think it's a limitation of the
language definition, nothing to do with .NET or otherwise, in the
context of "throwing an exception from the constructor".

But which limitation are you talking about exactly? That's what I don't
understand...

Yes, more hacks and crutches...

Huuh? Andrei Alexdandrescu doing "hacks and crutchs" ??? Care to justify
your appreciation?

More seriously, I believe there is a true divergence ni philosophy here :
The C++ approach is "do as little as possible in language itself, and as
much as possible in libraries", whereas your approach (I am not really sure
this is indeed the C# approach) is "put everything in language and compiler
itself, so that there is nothing left to do in libraries". I believe the 1st
approach is more flexible because, first, it makes it much easier to replace
a defective component....

Arnaud
MVP - VC

 

[Barry Kelly]

But which limitation are you talking about exactly? That's what I don't
understand...

Like I said, we'll have to agree to disagree.

More seriously, I believe there is a true divergence ni philosophy here :
The C++ approach is "do as little as possible in language itself, and as
much as possible in libraries",

Nonsense! If that were true, the language would look like LISP!

whereas your approach (I am not really sure
this is indeed the C# approach) is "put everything in language and compiler
itself, so that there is nothing left to do in libraries".

That's clearly not true, because there seem to be substantial libraries
packaged with the .NET framework. Even the C# basic types are defined
in a library!

You'll have to forgive me, I'm being quite facetious, at the end of a
long day.

I believe the 1st
approach is more flexible because, first, it makes it much easier to replace
a defective component....

Consider std::stack:op() - C++ is a language in which you can't even
write an exception-safe mutator method which returns a value of a
user-defined type!

I think a programming language should have a carefully selected set of
abstractions and abstraction creation tools that are suitable for the
domain to which it is applied. I think C++ has a set of useful
abstractions and abstraction creation tools, but there are interactions
between some of them that are less than desirable - and that the
resulting sum is somewhat less than the sum of its parts. I think it
would be a better language if it removed some features, and changed some
functionality - but then it would no longer be C++.

-- Barry