object.enable() anti-pattern

[Roy Smith]

I'd be curious to see in-the-wild instances of the anti-pattern that
you are talking about, then. I think everyone agrees that entirely
unmotivated "enable" methods should be avoided, but I have my doubts
that they come up very often.

As I mentioned earlier in this thread, this was a common pattern in the
early days of C++, when exceptions were a new concept and handled poorly
by many compilers (and, for that matter, programmers).

There was a school of thought that constructors should never be able to
fail (because the only way for a constructor to fail is to throw an
exception). The pattern was to always have the constructor succeed, and
then either have a way to check to see if the newly-constructed object
was valid, or have a separate post-construction initialization step
which could fail.

See, for example, the isValid() and Exists() calls for RogueWave's
RWFile class (http://tinyurl.com/c8kv26g). And also,
http://tinyurl.com/cgs6clx.

Even today, there are C++ implementations which do not use exceptions.
Some are for use in embedded or real-time systems where things need to
be strictly time-bound and/or memory-bound. Others are for historical
reasons (http://tinyurl.com/6hn4zo).

Once people were used to writing "can't fail" constructors in C++, they
often continued using that pattern in other languages, where the
underlying reasons no longer made sense. Quite possibly, they never
even knew the underlying reasons; they were taught, "Constructors must
never fail", and assumed it was a universal rule.

This, BTW, is one of my biggest beefs with the classic Gang Of Four
pattern book. It presents a bunch of patterns as being universally
applicable, when in reality many (if not most) of them are highly C++
specific.

BTW, whenever I read things like, "I think everyone agrees", I
automatically assume what the writer really meant was, "I, and all the
people who agree with me, think".

 

[Oscar Benjamin]

As I mentioned earlier in this thread, this was a common pattern in the
early days of C++, when exceptions were a new concept and handled poorly
by many compilers (and, for that matter, programmers).

There was a school of thought that constructors should never be able to
fail (because the only way for a constructor to fail is to throw an
exception). The pattern was to always have the constructor succeed, and
then either have a way to check to see if the newly-constructed object
was valid, or have a separate post-construction initialization step
which could fail.

It's not just because of exceptions. In C++ virtual method calls in a
constructor for a class A will always call the methods of class A even
if the object being constructed is actually of a subclass B because
the B part of the object isn't initialised when the A constructor is
called. There may be a better way to do this since I last used C++ but
as I remember it the two-phase pattern was a recommended way to
implement polymorphic behaviour during initialisation.


Oscar

 

[Roy Smith]

It's not just because of exceptions. In C++ virtual method calls in a
constructor for a class A will always call the methods of class A even
if the object being constructed is actually of a subclass B because
the B part of the object isn't initialised when the A constructor is
called. There may be a better way to do this since I last used C++ but
as I remember it the two-phase pattern was a recommended way to
implement polymorphic behaviour during initialisation.

Mind. Blown.

One of the things I love (FSVO love) about C++ is that no matter how
much I learn, there's always whole new areas of wonderment to explore
behind doors I didn't even know existed.

Thank you.

I suppose, if I had a class like this, I would write a factory function
which called the constructor and post-construction initializer. And
then I would make the constructor protected.

 

[Chris Angelico]

I suppose, if I had a class like this, I would write a factory function
which called the constructor and post-construction initializer. And
then I would make the constructor protected.

That sounds like a reasonable plan, with the possible exception of
protected. Since meeting Python, I've stopped using private and
protected anywhere.

ChrisA

 

[Roy Smith]

That sounds like a reasonable plan, with the possible exception of
protected. Since meeting Python, I've stopped using private and
protected anywhere.

ChrisA

Each language has its own set of best practices. Trying to write C++
code using Python patterns is as bad as trying to write Python code
using C++ patterns.

 

[Chris Angelico]

Each language has its own set of best practices. Trying to write C++
code using Python patterns is as bad as trying to write Python code
using C++ patterns.

Agreed, in generality. But what is actually gained by hiding data from
yourself? Compare:

class Foo
{
int asdf;
public:
Foo(int _asdf):asdf(_asdf) {}
int get_asdf() {return asdf;}
void set_asdf(int _asdf) {asdf=_asdf;}
void frob() {printf("Hi, I am %d\n",asdf);}
};

struct Foo
{
int asdf;
Foo(int _asdf):asdf(_asdf) {}
void frob() {printf("Hi, I am %d\n",asdf);}
};

Is there anything worse about the second one? Oh, and by logical
extension, here's something that doesn't (AFAIK) work in C++, but does
in another language that's syntactically similar:

class Foo(int asdf)
{
void frob() {write("Hi, I am %d\n",asdf);}
}

Now that's brevity. Why bother saying what's patently obvious? (Pike's
"class" keyword is like C++'s "struct", members are public by
default.)

ChrisA

 

[Roy Smith]

Each language has its own set of best practices. Trying to write C++
code using Python patterns is as bad as trying to write Python code
using C++ patterns.

Agreed, in generality. But what is actually gained by hiding data from
yourself?[/QUOTE]

You're not hiding it from yourself. You're hiding it from the other
people who are using your code and may not understand all the subtle
gotchas as well as you do.

In the calling-virtual-methods-in-the-constructor case we're talking
about here, constructing an object by calling B() without immediately
following it up with a call to B:ostInit() is dangerous. If you
document that you shouldn't do that, but allow it anyway, people will do
it. Better to disallow it completely. People will bitch when their
code doesn't compile, but that's better than compiling and running and
producing the wrong results.

You solve this problem in Python by simply having the constructor do the
right thing.

 

[Robert Kern]

As I mentioned earlier in this thread, this was a common pattern in the
early days of C++, when exceptions were a new concept and handled poorly
by many compilers (and, for that matter, programmers).

There was a school of thought that constructors should never be able to
fail (because the only way for a constructor to fail is to throw an
exception). The pattern was to always have the constructor succeed, and
then either have a way to check to see if the newly-constructed object
was valid, or have a separate post-construction initialization step
which could fail.

See, for example, the isValid() and Exists() calls for RogueWave's
RWFile class (http://tinyurl.com/c8kv26g). And also,
http://tinyurl.com/cgs6clx.

Even today, there are C++ implementations which do not use exceptions.
Some are for use in embedded or real-time systems where things need to
be strictly time-bound and/or memory-bound. Others are for historical
reasons (http://tinyurl.com/6hn4zo).

Once people were used to writing "can't fail" constructors in C++, they
often continued using that pattern in other languages, where the
underlying reasons no longer made sense. Quite possibly, they never
even knew the underlying reasons; they were taught, "Constructors must
never fail", and assumed it was a universal rule.

Right, this is one of the "bad reasons" I talk about later in my message. The
authors had a reason in their mind for doing it (they thought it was a universal
rule); it was just a bad one. It's more useful to talk about why people thought
they should follow that pattern than to just say "there is no reason to do this".

This, BTW, is one of my biggest beefs with the classic Gang Of Four
pattern book. It presents a bunch of patterns as being universally
applicable, when in reality many (if not most) of them are highly C++
specific.

BTW, whenever I read things like, "I think everyone agrees", I
automatically assume what the writer really meant was, "I, and all the
people who agree with me, think".

Hah! Fair enough. I actually meant it to emphasize that I thought that Steven
was overly reducing his statements to something that was trivially true,
sacrificing content for validity. I will suggest that your interpretation of
that phrase is more appropriate when the speaker is proposing something of their
own rather than (partially) conceding a point. The exaggeration is only
self-aggrandizing in the former case.

--
Robert Kern

"I have come to believe that the whole world is an enigma, a harmless enigma
that is made terrible by our own mad attempt to interpret it as though it had
an underlying truth."
-- Umberto Eco

 

[Chris Angelico]

You're not hiding it from yourself. You're hiding it from the other
people who are using your code and may not understand all the subtle
gotchas as well as you do.

True. And on looking over my code, I find that there are a few cases
where I've used private members: I have a buffer class that acts
pretty much like a non-refcounted string, and it declares a private
copy constructor to prevent accidental misuse. But it's the exception,
not the rule. My main point isn't about the cases where you actually
want to prevent access, but the all-too-common case where the member
itself is private and there are two public methods to get and set it.
Massive boilerplate. Completely unnecessary in 99%+ of cases.

ChrisA

 

[Serhiy Storchaka]

10.05.13 15:19, Robert Kern напиÑав(ла):

I'd be curious to see in-the-wild instances of the anti-pattern that you
are talking about, then.

Many (if not most) GUI frameworks use this pattern.

button = Button("text")
button.setForegroundColor(...)
button.setBackgoundColor(...)
button.setFont(...)
button.setRelief(...)
button.setBorder(...)
button.setWidth(...)
button.setAction(...)
button.setMouseListener(...)
button.place(...)

Another example is running a subprocess in Unix-like systems.

fork()
open/close file descriptors, set limits, etc
exec*()

 

[Nobody]

There is no sensible use-case for creating a file without opening it.
What would be the point? Any subsequent calls to just about any method
will fail. Since you have to open the file after creating the file object
anyway, why make them two different calls?

As a counterpoint, some OSes (e.g. Plan 9) allow you to get a "handle" to
a file without opening it. This can then be used for deleting, renaming or
stat()-type operations without either the risk of race conditions (if
another process renames files between operations, the operations may
be performed on different files) or the side-effects of actually opening
the file (particularly for device files, e.g. opening a tape drive may
rewind the tape).

Python's file model doesn't allow for this, so there isn't really anything
meaningful that you can do on a file object which isn't open (although
these actually exist; any file object on which the .close() method has
been called will be in this state).

However: there are situations where it is useful to be able to separate
the simple task of creating an object from more invasive actions such as
system calls. Particularly in multi-threaded or real-time code (although
the latter is a non-starter in Python for many other reasons).

 

[Robert Kern]

10.05.13 15:19, Robert Kern напиÑав(ла):

Many (if not most) GUI frameworks use this pattern.

button = Button("text")
button.setForegroundColor(...)
button.setBackgoundColor(...)
button.setFont(...)
button.setRelief(...)
button.setBorder(...)
button.setWidth(...)
button.setAction(...)
button.setMouseListener(...)
button.place(...)

Another example is running a subprocess in Unix-like systems.

fork()
open/close file descriptors, set limits, etc
exec*()

According to Steven's criteria, neither of these are instances of the
anti-pattern because there are good reasons they are this way. He is reducing
the anti-pattern to just those cases where there is no reason for doing so. That
is why I asked for in-the-wild instances. I should have qualified my sentence to
include "according to your criteria" because people seem to be answering my
query out of that context.

--
Robert Kern

"I have come to believe that the whole world is an enigma, a harmless enigma
that is made terrible by our own mad attempt to interpret it as though it had
an underlying truth."
-- Umberto Eco

 

[Chris Angelico]

10.05.13 15:19, Robert Kern напиÑав(ла):



Many (if not most) GUI frameworks use this pattern.

button = Button("text")
button.setForegroundColor(...)
button.setBackgoundColor(...)
button.setFont(...)
button.setRelief(...)
button.setBorder(...)
button.setWidth(...)
button.setAction(...)
button.setMouseListener(...)
button.place(...)

The button really exists, though. You could merge the creation and
placement (or in the case of a window/dialog, the creation and
showing), but it's often useful to not. However, in the specific case
you have there, there's an alternative: a mapping of attributes and
values passed to the constructor, and then you could pass the
constructed object directly to a place(). That would let you, if you
wished, effectively construct a Button with a parent right there,
which makes reasonable sense.

Another example is running a subprocess in Unix-like systems.

fork()
open/close file descriptors, set limits, etc
exec*()

Hrm. Not really a corresponding example. After you fork, you have two
actual processes. Following up with an exec is only one of the
possible options; I've done code that forks and execs, and code that
forks and keeps running, and neither of them feels "wrong" in any way.
There IS a function that's similar to what you're saying, and that's
vfork:

"""
(From POSIX.1) The vfork() function has the same effect as fork(2),
except that the behavior is undefined if the process created by
vfork() either modifies any data other than a variable of type pid_t
used to store the return value from vfork(), or returns from the
function in which vfork() was called, or calls any other function
before successfully calling _exit(2) or one of the exec(3) family of
functions.
"""

It's deprecated because it's so fragile (and because regular fork()
isn't that much less efficient now; AIUI, vfork was meant to be a
lightweight fork). I would say that the deprecation of vfork in favour
of fork is a strong indication that the object.enable() anti-pattern
can come up in kernel APIs too, and isn't liked there either.

ChrisA

 

[Roy Smith]

However: there are situations where it is useful to be able to separate
the simple task of creating an object from more invasive actions such as
system calls. Particularly in multi-threaded or real-time code (although
the latter is a non-starter in Python for many other reasons).

Sure. I can serialize a path name. I can't serialize an open file
descriptor.

 

[Cameron Simpson]

| In article <[email protected]>,
| > > int fd = 37;
| > >
| > > I've just created a file descriptor. There is not enough information
| > > given to know if it corresponds to an open file or not.
| >
| > No, you haven't created a file descriptor. You've made up a number which
| > C will allow you to use as an index into the file descriptor table,
| > because C is a high-level assembler with very little in the way of type
| > safety, and what little there is you can normally bypass.
|
| No, I've created a file descriptor, which is, by definition, an integer.
| It has nothing to do with C. This is all defined by the POSIX
| interface. For example, the getdtablesize(2) man page says:
|
| "The entries in the descriptor table are numbered with small integers
| starting at 0. The call getdtablesize() returns the size of this table."
[... snip ...]

I'm with Steven here.

You've made a number that can be used with calls that access the
OS file descriptor table. But it isn't a file descriptor. (Yes, the
in-program number is just a number either way.)

The descriptor table is an in-kernel data structure, filled with
file descriptors. All you have is a label that may or may not access
a file descriptor.

Anyway, we all know _what_ goes on. We're just having terminology issues.

Cheers,
--
Cameron Simpson <[email protected]>

My computer always does exactly what I tell it to do but sometimes I have
trouble finding out what it was that I told it to do.
- Dick Wexelblat <[email protected]>

 

[Mark Janssen]

| No, I've created a file descriptor, which is, by definition, an integer.

| It has nothing to do with C. This is all defined by the POSIX
| interface. For example, the getdtablesize(2) man page says:
|
| "The entries in the descriptor table are numbered with small integers
| starting at 0. The call getdtablesize() returns the size of this table."
[... snip ...]

I'm with Steven here.

You've made a number that can be used with calls that access the
OS file descriptor table. But it isn't a file descriptor. (Yes, the
in-program number is just a number either way.)

Steven, don't be misled. POSIX is not the model to look to -- it does
not acknowledge that files are actual objects that reside on a piece
of hardware. It is not simply an integer.

Mark

 

[Mark Janssen]

Steven, don't be misled. POSIX is not the model to look to -- it does

not acknowledge that files are actual objects that reside on a piece
of hardware. It is not simply an integer.

Please disregard this (my own) flame bait.

 

[Steven D'Aprano]

As a counterpoint, some OSes (e.g. Plan 9) allow you to get a "handle"
to a file without opening it. This can then be used for deleting,
renaming or stat()-type operations without either the risk of race
conditions (if another process renames files between operations, the
operations may be performed on different files) or the side-effects of
actually opening the file (particularly for device files, e.g. opening a
tape drive may rewind the tape).

Ah, now that's a fantastic counter-example. But I think that says more
about the primitiveness of the Unix file model than of the idea of
temporal coupling.

Python's file model doesn't allow for this, so there isn't really
anything meaningful that you can do on a file object which isn't open
(although these actually exist; any file object on which the .close()
method has been called will be in this state).

Absolutely correct, and I'm amazed it's taken this long for anyone to
point this out

 

[Steven D'Aprano]

According to Steven's criteria, neither of these are instances of the
anti-pattern because there are good reasons they are this way. He is
reducing the anti-pattern to just those cases where there is no reason
for doing so.

But isn't that the case for all anti-patterns?

We agree that GOTO is an anti-pattern. That doesn't mean that there
aren't valid reasons for using GOTO. Sometimes there are good use-cases
for GOTO that outweigh the harm. By calling it an anti-pattern, though,
we shift the onus onto the person wanting to use GOTO: justify why you
need it, or use something else.

Would you object less if I called it a "code smell" than an "anti-
pattern"? If so, I accept your criticism, and call it a code smell: the
less temporal coupling your API has, the better.

That is why I asked for in-the-wild instances.

How about this?

http://legacy.thecodewhisperer.com/post/366626867/improving-clarity-by-removing-temporal-coupling


Another example of temporal coupling is json_decode in PHP: you must
follow it by a call to json_last_error, otherwise you have no way of
telling whether the json_decode function succeeded or not.

http://php.net/manual/en/function.json-last-error.php

I should
have qualified my sentence to include "according to your criteria"
because people seem to be answering my query out of that context.

Now you know how I feel

I started this thread asking for help tracking down a blog post
describing this code smell, or at least the name of such. Thanks again to
Wayne Werner, who came through with the name of the anti-pattern,
temporal coupling, and a blog post describing it, although not the one I
read all those many moons ago.

I never intended to give the impression that *any* use of a separate
"enable" method call was bad. I certainly didn't intend to be bogged
down into a long discussion about the minutia of file descriptors in
C, but it was educational

 

[Roy Smith]

I never intended to give the impression that *any* use of a separate
"enable" method call was bad. I certainly didn't intend to be bogged
down into a long discussion about the minutia of file descriptors in
C, but it was educational

Well, you did say you were here for abuse. I think you got your money's
worth. Can I interest you in a course of getting hit on the head
lessons?

And just to be clear to the studio audience and all of you who are
watching at home...

For all the effort I put into nit-picking, I do agree with Steven's
basic premise. Two-phase construction is usually not the right way to
be designing classes. Especially in languages like Python where
constructors raising exceptions is both inexpensive and universally
accepted as normal behavior.