Name that anti-pattern

Discussion in 'Java' started by Antti S. Brax, Sep 29, 2005.

  1. 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.
     
    Antti S. Brax, Sep 29, 2005
    #1
    1. Advertisements

  2. Your example breaks for me - as InputStream.available() exists.
     
    Thomas Schodt, Sep 29, 2005
    #2
    1. Advertisements

  3. Antti S. Brax

    Roedy Green Guest

    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.
     
    Roedy Green, Sep 29, 2005
    #3
  4. It breaks for you because you don't know what available()
    does.
     
    Antti S. Brax, Sep 29, 2005
    #4
  5. Antti S. Brax

    iamfractal Guest

    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
     
    iamfractal, Sep 29, 2005
    #5
  6. 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.
     
    Thomas Schodt, Sep 29, 2005
    #6
  7. Antti S. Brax

    Chris Smith Guest

    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
     
    Chris Smith, Sep 29, 2005
    #7
  8. 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.
    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, Sep 30, 2005
    #8
  9. 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."
     
    Antti S. Brax, Sep 30, 2005
    #9
  10. 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...
     
    Thomas Schodt, Sep 30, 2005
    #10
  11. 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.
     
    Antti S. Brax, Sep 30, 2005
    #11
  12. Antti S. Brax

    Chris Uppal Guest

    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
     
    Chris Uppal, Sep 30, 2005
    #12
  13. 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.
     
    Thomas Schodt, Sep 30, 2005
    #13
  14. That is correct. I'm ashamed that I didn't figure it out myself.
     
    Antti S. Brax, Sep 30, 2005
    #14
  15. Antti S. Brax

    Chris Smith Guest

    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 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, Sep 30, 2005
    #15
  16. Antti S. Brax

    Chris Smith Guest

    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
     
    Chris Smith, Sep 30, 2005
    #16
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.