Name that anti-pattern

[Antti S. Brax]

I noticed recently something that could qualify as an anti
pattern (it's an instance of "designing for the future"). Please
comment.

I have a class which is not constrained by any interface. It has
a method which returns an InputStream. The implementation of the
class always returns a ByteArrayInputStream. Because the
designer of the class does not let me know the full capabilities
of the returned object I have to treat the returned object as a
plain InputStream and cannot take advantage of the information
that comes with a ByteArrayInputStream (mainly the available()
method).

Because of this I either have to use a lot of instanceof or end
up with an inefficient solution.

The designer of the class prepared for the situation where his
own implementation changes. The implementation never changed.
Poorly performing code resulted.

 

[Thomas Schodt]

I have a class which is not constrained by any interface. It has
a method which returns an InputStream. The implementation of the
class always returns a ByteArrayInputStream. Because the
designer of the class does not let me know the full capabilities
of the returned object I have to treat the returned object as a
plain InputStream and cannot take advantage of the information
that comes with a ByteArrayInputStream (mainly the available()
method).

Your example breaks for me - as InputStream.available() exists.

 

[Roedy Green]

ByteArrayInputStream (mainly the available()

In a similar way getURLConnection will return an HTTPConnection. If
you KNOW that absolutely, you can cast the result to an HTTPConnection
and use the extended methods.

If you were wrong, you will get a cast exception.

 

[Antti S. Brax]

Your example breaks for me - as InputStream.available() exists.

It breaks for you because you don't know what available()
does.

 

[iamfractal]

Interesting.

I don't know what available() does either, but I'd be surprised if its
behaviour changes just because its object is re-cast.

Of course, I must be missing something here.

Maybe it's an antti-pattern

..ed

 

[Thomas Schodt]

It breaks for you because you don't know what available()
does.

Returns a positive non-zero value if a read() posted now can be
guaranteed to be able to read that many bytes without blocking.

Returns 0 (zero) if no guarantees can be made
that a read() posted now would not block or return EOF.


Anyway, I am sure you know that
*if* the object happens to be a ByteArrayInputStream,
you *will* get the result of ByteArrayInputStream.available(),

so I'm struggling to see what the problem is.

The javadoc for InputStream says
public abstract class InputStream
The available() method for class InputStream always returns 0.
This method should be overridden by subclasses.

You will always be dealing with a subclass and the subclass should
always override the available() method.

 

[Chris Smith]

Interesting.

I don't know what available() does either, but I'd be surprised if its
behaviour changes just because its object is re-cast.

Of course, I must be missing something here.

Sure. The ByteArrayInputStream class fulfills all the requirements of
the superclass (i.e., it returns the number of bytes that can be read
without blocking)... but it also makes the additional guarantee that it
returns the number of bytes remaining in the complete stream. Antti
wants to use that guarantee... and although the InputStream would act
that way without a cast, it would be very wrong to rely on the
guarantees made by a ByteArrayInputStream without verifying that the
reference is of that type.

I don't see this as an anti-pattern. The original author made a trade-
off between flexibility and performance. If Antti doesn't like it, then
fine. But that doesn't mean that we can categorize that whole family of
decisions as being a bad idea.

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Antti S. Brax]

I don't see this as an anti-pattern. The original author made a trade-
off between flexibility and performance.

But think for a second what flexibility was traded off? Surely a
ByteArrayInputStream can be used where an InputStream is needed,
but not vice versa. So no flexibility for the class user was
added (quite the opposite, flexibility was lost). The reason why
the author used InputStream was because he expected the
implementation to change (which never happened). In this question
changing the implementation to something else would have required
a massive redesign.

Or then the author was just having "OOP blindness". In other
words, used InputStream because every _interface_ he has seen
uses it.

But that doesn't mean that we can categorize that whole family of
decisions as being a bad idea.

Please describe the "family of decisions" you are talking about.
I think you understood me incorrectly and would like to correct
it.

 

[Antti S. Brax]

Anyway, I am sure you know that
*if* the object happens to be a ByteArrayInputStream,
you *will* get the result of ByteArrayInputStream.available(),

so I'm struggling to see what the problem is.

Chris Smith just described it very well: "it also makes the
additional guarantee that it returns the number of bytes
remaining in the complete stream."

 

[Thomas Schodt]

The ByteArrayInputStream class fulfills all the requirements of
the superclass (i.e., it returns the number of bytes that can be read
without blocking)... but it also makes the additional guarantee that it
returns the number of bytes remaining in the complete stream. Antti
wants to use that guarantee...

I cannot find that guarantee in the Javadoc.
Yes, perusing the source, it is obvious it is there.

In this case it is highly unlikely you would ever come across an
implementation that does not make the same guarantee,
but relying on undocumented implementation specific behaviour
can often come back and bite you.
Almost makes me think it might be an anti-pattern...

 

[Antti S. Brax]

I cannot find that guarantee in the Javadoc.
Yes, perusing the source, it is obvious it is there.

In this case it is highly unlikely you would ever come across an
implementation that does not make the same guarantee,
but relying on undocumented implementation specific behaviour
can often come back and bite you.
Almost makes me think it might be an anti-pattern...

Well, your inability to understand documentation could also
be thought as an anti-pattern.


available(): The value returned is count - pos, which is the
number of bytes remaining to be read from the input buffer.

count: The index one greater than the last valid character in
the input stream buffer.

pos: The index of the next character to read from the input
stream buffer.

I think it is quite clearly stated in the documentation.

 

[Chris Uppal]

I have a class which is not constrained by any interface. It has
a method which returns an InputStream. The implementation of the
class always returns a ByteArrayInputStream. Because the
designer of the class does not let me know the full capabilities
of the returned object I have to treat the returned object as a
plain InputStream and cannot take advantage of the information
that comes with a ByteArrayInputStream (mainly the available()
method).

It's either:
poor API design
or
poor coding on your part.

if it's inherent to the class's function that it hands out a
ByteArrayInputStream from which you read, then why not just make the class hand
out the completely filled-in ByteArray instead ? Much simpler, faster, and
more flexible.

If, on the other hand, it is /not/ fundamental to how the thing works, and that
the class designer cannot asume that it will remain true that all the data can
be held in memory at once; then you are misusing the API by second-guessing the
implementation. (And there is little or no valid use for available() anyway).

If you legitmately need to be able to take advantage of the case where the
input stream /is/ fully buffered, then Java provides the ideal solution -- use
a cast. And then write code to handle the error case.

BTW, in the rare cases where the kind of pattern you descibe does legimately
occur (and I don't think your example is one) then I'd call it "hiding too much
information".

-- chris

 

[Thomas Schodt]

your inability to understand documentation could also
be thought as an anti-pattern.

Amazing.
It is beyond belief that I looked at the entry several times
and completely missed what it was saying.

I had it in front of me and had to ask the browser to search for "The
value returned"...
Talk about a blind spot.

 

[Antti S. Brax]

It's either:
poor API design
or
poor coding on your part.

if it's inherent to the class's function that it hands out a
ByteArrayInputStream from which you read, then why not just make the class
hand out the completely filled-in ByteArray instead ? Much simpler, faster,
and more flexible.

That is correct. I'm ashamed that I didn't figure it out myself.

 

[Chris Smith]

But think for a second what flexibility was traded off?

Flexibility in the implementation of that declared API. Whether that
flexibility is required is a decision that involves various trade-offs.
I tend to side with Chris Uppal's statement though: it would make sense
to return InputStream or to return byte[], but I can't imagine it being
appropriate to return a ByteArrayInputStream.

The reason why
the author used InputStream was because he expected the
implementation to change (which never happened).
Yet.

Please describe the "family of decisions" you are talking about.
I think you understood me incorrectly and would like to correct
it.

The family could be described as decisions that withhold information
from the client of an API in order to preserve the ability to change the
implementation later. That's what you're talking about, right?

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation

 

[Chris Smith]

Anyway, I am sure you know that
*if* the object happens to be a ByteArrayInputStream,
you *will* get the result of ByteArrayInputStream.available(),

so I'm struggling to see what the problem is.

If I have an InputStream, it is incorrect to try to read the entire
stream to EOF by writing this code:

InputStream in;
byte[] b = new byte[in.available()];
for (int i = 0; i < b.length; i++) b = (byte) in.read();

That code will work fine if the stream happens to be an instance of
ByteArrayInputStream. However, making that unchecked assumption is
fragile. Since the code relies on behavior that is unique to the
subclass, the reference 'in' should be of type ByteArrayInputStream.

Next year, when the API is reimplemented to use a network service, that
InputStream -- whose concrete class isn't guaranteed by the API -- may
turn into a SocketInputStream instead. The code will break. Someone
will chase the bug until they determine that there's an assumption that
available() will return the entire remaining length of the stream. They
will wonder about your sanity and curse your name as they fix the bug.

And therein lies the problem. I don't mind when others wonder about my
sanity, but it's not fun to have people curse my name.

Now, there are several solutions, and what follows is an incomplete
list:

1. Cast the reference to ByteArrayInputStream before the code above.
This ensures two things: a) that the runtime will complain properly if
your assumption is violated; and b) that readers will realize your
assumption.

2. Write the code so that it works for any InputStream. There's some
expense to reallocating and copying the array contents as you work, but
it's unlikely to change the asymptotic complexity of any time-critical
algorithms, since the operation of building the array still runs in the
comparatively simple O(n*log[n]) time if written properly.

3. Test with instanceof and write both an optimized version for
ByteArrayInputStream and a general-purpose one for InputStream. This is
probably not a good idea.

I still don't think that the interface is necessarily bad or wrong, but
that's the concern anyway.

--
www.designacourse.com
The Easiest Way To Train Anyone... Anywhere.

Chris Smith - Lead Software Developer/Technical Trainer
MindIQ Corporation