When designing a class should consistency in behaviour be favoured over common programming practice? To give a specific example:

A common convention is this: If a class owns an object (e.g. it created it) it is responsible of cleaning it up once it's done. A specific example would be in .NET that if your class owns an IDisposable object it should dispose it at the end of its life. And if you don't own it then don't touch it.

Now if we look at the StreamWriter class in .NET then we can find in the documentation that it closes the underlying stream when it is being closed/disposed. This is necessary in cases where the StreamWriter is instantiated by passing in a file name as writer creates the underlying file stream and therefore needs to close it. However one can also pass in an external stream which the writer also closes.

This has annoyed me heaps of times (yes I know you can make a non-closing wrapper but that's not the point) but apparently Microsoft has made the decision that it's more consistent to always close the stream no matter where it came from.

When I come accross such a pattern in one of my classes I usually create a ownsFooBar flag which gets set to false in cases where FooBar is injected via the constructor and to true otherwise. This way the responsibility of cleaning it up is passed to the caller when he passes the instance explicitly.

Now I'm wondering if maybe consistency should be in favour over best practice (or maybe my best practice is not that good)? Any arguments for/against it?

Edit for clarification

With "consistency" I mean: The consistent behaviour of the class always taking ownership (and closing the stream) vs "best practice" to only take ownership of a object if you created it or explicitly transfered the ownership.

As for an example where it is enoying:

Assume you have two given classes (from some 3rd party library) which accept a stream to do something with it, like creating and processing some data:

This will fail with an exception Cannot access a closed stream in the data processor. It's a bit constructed but should illustrate the point. There are various ways to fix it but nevertheless I feel that I work around something I should not have to.

4 Answers
4

I use this technique too, I call it 'handoff', and I believe it deserves the status of a pattern. When an object A accepts a disposable object B as a construction time parameter, it also accepts a boolean called 'handoff', (which defaults to false,) and if it is true, then disposal of A cascades to disposal of B.

I am not in favor of proliferating other people's unfortunate choices, so I would never accept Microsoft's bad practice as an established convention, nor would I consider other people's blind following of Microsoft's unfortunate choices to be "consistent behaviour" in any way, shape or form.

As a further extension of the handOff pattern, if such a property is set in the constructor, but another method exists to modify it, that other method should also accept a handOff flag. If handOff was specified for the presently-held item, it should be disposed, then the new item should be accepted and the internal handOff flag updated to reflect the new item's status. The method shouldn't need to check the old and new references for equality, since if the original reference was "handed off" all references held by the original caller should be abandoned.
– supercatJul 15 '14 at 18:39

@supercat I concur, but I have an issue with the last sentence: if handoff is true for the presently-held item, but the newly-supplied item is equal by reference to the presently-held item, then disposing the presently-held item is in effect disposing the newly-supplied item. I think that re-supplying the same item should be illegal.
– Mike NakisJul 15 '14 at 19:44

Re-supplying the same item should generally be illegal unless the object is immutable, doesn't actually hold any resources, and doesn't care if it's disposed. For example, if Dispose requests were ignored for system brushes, a "color.CreateBrush` method might at its leisure either create a new brush (which would require disposal) or return a system brush (which would ignore disposal). The simplest way to prevent reuse of an object if it cares about disposal, but allow reuse if it doesn't, is to dispose it.
– supercatJul 15 '14 at 21:02

I'd go with consistency. As you mentioned to most people, it makes sense that if your object creates a child object, it will own it and destroy it. If it is given a reference from the outside, at some point in time "the outside" also is holding the same object and it shouldn't be owned. If you want to transfer ownership from the outside into your object use Attach/Detach methods or a constructor which accepts a boolean that makes it clear that ownership is transferred.

Having typed all that for complete answer, I think most generic design patterns wouldn't necessarily fall into the same category as StreamWriter/StreamReader classes. I've always thought that the reason MS chose to have StreamWriter/Reader automatically take over the ownership is because in this specific instance, having shared ownership doesn't make a lot of sense. You can't have two different objects concurrently read from/write to the same stream. I'm sure you could write some kind of deterministic time share, but that would be one hell of an anti-pattern. So that's probably why they said, since it makes no sense to share a stream, let's just always take it over. But I wouldn't consider this behavior to ever have become a generic convention across the board for all classes.

You could consider Streams to be a consistent pattern where the object that is being passed in is non-sharable from design point of view and therefore without any additional flags, the reader/writer just takes over its ownership.

Be careful. What you think of as "consistency" might just be a random collection of habits.

"Coordinating user interfaces for consistency", SIGCHI Bulletin 20, (1989), 63-65. Sorry, no link, it's an old article. "... a two-day workshop of 15 experts was unable to produce a definition of consistency." Yes, it's about "interface", but I believe that if you think about "consistency" for a while, you'll find you can't define it, either.

you are right, if the experts come together from different circles, but when developing code, I found it easy to follow some existing pattern (or habit if you will) which my team is already familiar with. For example, the other day one of our guys was inquiring about some asynchronous operations we had and when I told him they behave the same way as Win32 Overlapped I/O, in 30 seconds he understood the entire interface... in this case both myself and him came from the same server back end Win32 background. Consistency ftw! :)
– DXMJan 12 '12 at 2:58

@Bruce I get your point - I thought it was clear what I meant with consistency but apparently it wasn't.
– ChrisWueJan 12 '12 at 6:43