You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Emmanuel Lecharny <el...@apache.org> on 2008/05/23 04:41:05 UTC

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Trustin,

can you add at least a simple Javadoc for every public method before 
committing ?

Committing code omitting javadoc is just a bad habit : you will have to 
add the Javadoc anyway later, but it will cost you more effort and more 
pain than doing it right away.


Thanks !

trustin@apache.org wrote:
> Author: trustin
> Date: Thu May 22 19:35:19 2008
> New Revision: 659372
>
> URL: http://svn.apache.org/viewvc?rev=659372&view=rev
> Log:
> Added the default implementation of ByteBufferQueue - need to write a test case
>
> Added:
>     mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java   (with props)
>
> Added: mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java
> URL: http://svn.apache.org/viewvc/mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java?rev=659372&view=auto
> ==============================================================================
> --- mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java (added)
> +++ mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java Thu May 22 19:35:19 2008
> @@ -0,0 +1,854 @@
> +/*
> + *  Licensed to the Apache Software Foundation (ASF) under one
> + *  or more contributor license agreements.  See the NOTICE file
> + *  distributed with this work for additional information
> + *  regarding copyright ownership.  The ASF licenses this file
> + *  to you under the Apache License, Version 2.0 (the
> + *  "License"); you may not use this file except in compliance
> + *  with the License.  You may obtain a copy of the License at
> + *
> + *    http://www.apache.org/licenses/LICENSE-2.0
> + *
> + *  Unless required by applicable law or agreed to in writing,
> + *  software distributed under the License is distributed on an
> + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + *  KIND, either express or implied.  See the License for the
> + *  specific language governing permissions and limitations
> + *  under the License.
> + *
> + */
> +package org.apache.mina.queue;
> +
> +import java.nio.ByteBuffer;
> +import java.nio.ByteOrder;
> +import java.util.ConcurrentModificationException;
> +import java.util.Iterator;
> +import java.util.NoSuchElementException;
> +import java.util.Queue;
> +
> +/**
> + * The default {@link ByteBufferQueue} implementation.
> + *
> + * @author The Apache MINA project (dev@mina.apache.org)
> + * @version $Rev$, $Date$
> + */
> +public class DefaultByteBufferQueue extends AbstractIoQueue<ByteBuffer> implements
> +        ByteBufferQueue {
> +
> +    private static final int DEFAULT_CAPACITY_INCREMENT = 512;
> +
> +    private final Queue<ByteBuffer> queue;
> +    private final ByteOrder order;
> +    private final ByteBufferFactory bufferFactory;
> +    private final int capacityIncrement;
> +    private ByteBuffer tail;
> +    private int length;
> +
> +    public DefaultByteBufferQueue() {
> +        this(
> +                new CircularQueue<ByteBuffer>(), ByteOrder.BIG_ENDIAN,
> +                HeapByteBufferFactory.INSTANCE, DEFAULT_CAPACITY_INCREMENT);
> +    }
> +
> +    public DefaultByteBufferQueue(int capacityIncrement) {
> +        this(
> +                new CircularQueue<ByteBuffer>(), ByteOrder.BIG_ENDIAN,
> +                HeapByteBufferFactory.INSTANCE, capacityIncrement);
> +    }
> +
> +    public DefaultByteBufferQueue(ByteOrder order) {
> +        this(
> +                new CircularQueue<ByteBuffer>(), order,
> +                HeapByteBufferFactory.INSTANCE, DEFAULT_CAPACITY_INCREMENT);
> +    }
> +
> +    public DefaultByteBufferQueue(ByteOrder order, int capacityIncrement) {
> +        this(
> +                new CircularQueue<ByteBuffer>(), order,
> +                HeapByteBufferFactory.INSTANCE, capacityIncrement);
> +    }
> +
> +    public DefaultByteBufferQueue(
> +            Queue<ByteBuffer> queue, ByteOrder order,
> +            ByteBufferFactory bufferFactory, int capacityIncrement) {
> +
> +        if (queue == null) {
> +            throw new NullPointerException("queue");
> +        }
> +        if (order == null) {
> +            throw new NullPointerException("order");
> +        }
> +        if (bufferFactory == null) {
> +            throw new NullPointerException("bufferFactory");
> +        }
> +        if (capacityIncrement < 8) {
> +            throw new IllegalArgumentException(
> +                    "capacityIncrement: " + capacityIncrement +
> +                    " (expected: an integer greater than or equals to 8)");
> +        }
> +
> +        this.queue = queue;
> +        this.order = order;
> +        this.bufferFactory = bufferFactory;
> +        this.capacityIncrement = capacityIncrement;
> +    }
> +
> +    @Override
> +    protected boolean doOffer(ByteBuffer e) {
> +        e = e.duplicate();
> +
> +        // Refuse to add an empty buffer.
> +        if (!e.hasRemaining()) {
> +            return false;
> +        }
> +
> +        tail = null;
> +        e.order(order);
> +        return queue.offer(e);
> +    }
> +
> +    @Override
> +    protected ByteBuffer doPoll() {
> +        ByteBuffer buf = queue.poll();
> +        if (buf == tail) {
> +            tail = null;
> +        }
> +        return buf;
> +    }
> +
> +    @Override
> +    public Iterator<ByteBuffer> iterator() {
> +        final Iterator<ByteBuffer> i = queue.iterator();
> +        return new Iterator<ByteBuffer>() {
> +            public boolean hasNext() {
> +                return i.hasNext();
> +            }
> +
> +            public ByteBuffer next() {
> +                return i.next();
> +            }
> +
> +            public void remove() {
> +                throw new UnsupportedOperationException();
> +            }
> +        };
> +    }
> +
> +    @Override
> +    public int size() {
> +        return queue.size();
> +    }
> +
> +    public ByteBuffer peek() {
> +        return queue.peek();
> +    }
> +
> +    public ByteOrder order() {
> +        return order;
> +    }
> +
> +    public int length() {
> +        return length;
> +    }
> +
> +    public void addByte(byte value) {
> +        if (!offerByte(value)) {
> +            throw new IllegalStateException();
> +        }
> +    }
> +
> +    public void addShort(short value) {
> +        if (!offerShort(value)) {
> +            throw new IllegalStateException();
> +        }
> +    }
> +
> +    public void addInt(int value) {
> +        if (!offerInt(value)) {
> +            throw new IllegalStateException();
> +        }
> +    }
> +
> +    public void addLong(long value) {
> +        if (!offerLong(value)) {
> +            throw new IllegalStateException();
> +        }
> +    }
> +
> +    public void addFloat(float value) {
> +        if (!offerFloat(value)) {
> +            throw new IllegalStateException();
> +        }
> +    }
> +
> +    public void addDouble(double value) {
> +        if (!offerDouble(value)) {
> +            throw new IllegalStateException();
> +        }
> +    }
> +
> +    public boolean offerByte(byte value) {
> +        ByteBuffer tail = tail(1);
> +        if (tail == null) {
> +            return false;
> +        }
> +        int index = tail.limit();
> +        tail.limit(index + 1);
> +        tail.put(index, value);
> +        length ++;
> +        return true;
> +    }
> +
> +    public boolean offerShort(short value) {
> +        ByteBuffer tail = tail(2);
> +        if (tail == null) {
> +            return false;
> +        }
> +        int index = tail.limit();
> +        tail.limit(index + 2);
> +        tail.putShort(index, value);
> +        length += 2;
> +        return true;
> +    }
> +
> +    public boolean offerInt(int value) {
> +        ByteBuffer tail = tail(4);
> +        if (tail == null) {
> +            return false;
> +        }
> +        int index = tail.limit();
> +        tail.limit(index + 4);
> +        tail.putInt(index, value);
> +        length += 4;
> +        return true;
> +    }
> +
> +    public boolean offerLong(long value) {
> +        ByteBuffer tail = tail(8);
> +        if (tail == null) {
> +            return false;
> +        }
> +        int index = tail.limit();
> +        tail.limit(index + 8);
> +        tail.putLong(index, value);
> +        length += 8;
> +        return true;
> +    }
> +
> +    public boolean offerFloat(float value) {
> +        return offerInt(Float.floatToIntBits(value));
> +    }
> +
> +    public boolean offerDouble(double value) {
> +        return offerLong(Double.doubleToLongBits(value));
> +    }
> +
> +    public boolean pollSlice(Queue<ByteBuffer> destination, int length) {
> +        if (length < 0) {
> +            throw new IllegalArgumentException(
> +                    "length: " + length +
> +                    " (expected: zero or a positive integer)");
> +        }
> +
> +        if (length > this.length) {
> +            return false;
> +        }
> +
> +        if (length == 0) {
> +            return true;
> +        }
> +
> +        int bytesToRead = length;
> +        for (;;) {
> +            ByteBuffer element = queue.peek();
> +            if (element == null) {
> +                // element shouldn't be null unless it's accessed concurrently.
> +                throw new ConcurrentModificationException();
> +            }
> +
> +            int remaining = element.remaining();
> +            if (remaining == 0) {
> +                removeAndAssert(element);
> +                continue;
> +            }
> +
> +            if (remaining >= bytesToRead) {
> +                int position = element.position();
> +                ByteBuffer lastElement = element.duplicate();
> +                lastElement.limit(position + bytesToRead);
> +                if (destination.offer(lastElement)) {
> +                    element.position(position + bytesToRead);
> +                    this.length -= bytesToRead;
> +                    return true;
> +                } else {
> +                    return false;
> +                }
> +            }
> +
> +            if (destination.offer(element.duplicate())) {
> +                removeAndAssert(element);
> +                element.limit(element.limit());
> +                this.length -= remaining;
> +                bytesToRead -= remaining;
> +            } else {
> +                return false;
> +            }
> +        }
> +    }
> +
> +    public boolean peekSlice(Queue<ByteBuffer> destination, int length) {
> +        if (length < 0) {
> +            throw new IllegalArgumentException(
> +                    "length: " + length +
> +                    " (expected: zero or a positive integer)");
> +        }
> +
> +        if (length > this.length) {
> +            return false;
> +        }
> +
> +        // No need to fetch anything if the specified length is zero.
> +        if (length == 0) {
> +            return true;
> +        }
> +
> +        // Optimize when it's enough with one slice.
> +        ByteBuffer element = safeElement();
> +        if (element.remaining() >= length) {
> +            ByteBuffer lastElement = element.duplicate();
> +            lastElement.limit(element.position() + length);
> +            return destination.offer(lastElement);
> +        }
> +
> +        // Otherwise we have to use an iterator.
> +        int bytesToRead = length;
> +        for (ByteBuffer e: this) {
> +            int remaining = e.remaining();
> +            if (remaining == 0) {
> +                continue;
> +            }
> +
> +            if (remaining >= bytesToRead) {
> +                ByteBuffer lastElement = element.duplicate();
> +                lastElement.limit(e.position() + bytesToRead);
> +                if (!destination.offer(lastElement)) {
> +                    return false;
> +                } else {
> +                    return true;
> +                }
> +            }
> +
> +            if (!destination.offer(element)) {
> +                return false;
> +            }
> +            bytesToRead -= remaining;
> +        }
> +
> +        // The only case that we reach here is concurrent access.
> +        throw new ConcurrentModificationException();
> +    }
> +
> +    public boolean elementAsSlice(Queue<ByteBuffer> destination, int length) {
> +        checkSequentialAccess(length);
> +        return peekSlice(destination, length);
> +    }
> +
> +    public boolean removeSlice(Queue<ByteBuffer> destination, int length) {
> +        checkSequentialAccess(length);
> +        return pollSlice(destination, length);
> +    }
> +
> +    public boolean getSlice(Queue<ByteBuffer> destination, int byteIndex, int length) {
> +        checkRandomAccess(byteIndex, length);
> +
> +        // No need to fetch anything if the specified length is zero.
> +        if (length == 0) {
> +            return true;
> +        }
> +
> +        if (byteIndex == 0) {
> +            return elementAsSlice(destination, length);
> +        }
> +
> +        // Optimize when it's enough with one slice.
> +        ByteBuffer element = safeElement();
> +        if (element.remaining() >= byteIndex + length) {
> +            ByteBuffer lastElement = element.duplicate();
> +            lastElement.position(lastElement.position() + byteIndex);
> +            lastElement.limit(lastElement.position() + length);
> +            return destination.offer(lastElement);
> +        }
> +
> +        // Otherwise we have to use an iterator.
> +        int bytesToRead = length;
> +        for (ByteBuffer b: this) {
> +            int remaining = b.remaining();
> +
> +            // Skip until we find the element which matches to the offset.
> +            if (remaining <= byteIndex) {
> +                byteIndex -= remaining;
> +                continue;
> +            }
> +
> +            if (remaining > byteIndex + length) {
> +                ByteBuffer lastElement = b.duplicate();
> +                lastElement.position(lastElement.position() + byteIndex);
> +                lastElement.limit(lastElement.position() + length);
> +                return destination.offer(lastElement);
> +            }
> +
> +            b = b.duplicate();
> +            b.position(b.position() + byteIndex);
> +            destination.offer(b);
> +            bytesToRead -= remaining - byteIndex;
> +            byteIndex -= remaining - byteIndex;
> +        }
> +
> +        throw new ConcurrentModificationException();
> +    }
> +
> +    public byte   removeByte() {
> +        checkSequentialAccess(1);
> +        ByteBuffer e = safeElement();
> +        length --;
> +        byte value = e.get();
> +
> +        if (!e.hasRemaining()) {
> +            trim();
> +        }
> +        return value;
> +    }
> +
> +    public short  removeShort() {
> +        checkSequentialAccess(2);
> +        // Try to read in one shot.
> +        ByteBuffer e = safeElement();
> +        int remaining = e.remaining();
> +        switch (remaining) {
> +        case 0: case 1:
> +            break;
> +        case 2:
> +            length -= 2;
> +            short value = e.getShort();
> +            trim();
> +            return value;
> +        default:
> +            length -= 2;
> +            return e.getShort();
> +        }
> +
> +        // Otherwise, read byte by byte. (inefficient!)
> +        int value = 0;
> +        for (int bytesToRead = 2; bytesToRead > 0; bytesToRead --) {
> +            value = value << 8 | e.get();
> +            remaining --;
> +            length --;
> +            if (remaining == 0) {
> +                e = safeElement();
> +                remaining = e.remaining();
> +            }
> +        }
> +
> +        return applyByteOrder((short) value);
> +    }
> +
> +    public int    removeInt() {
> +        checkSequentialAccess(4);
> +        // Try to read in one shot.
> +        ByteBuffer e = safeElement();
> +        int remaining = e.remaining();
> +        switch (remaining) {
> +        case 0: case 1: case 2: case 3:
> +            break;
> +        case 4:
> +            length -= 4;
> +            int value = e.getInt();
> +            trim();
> +            return value;
> +        default:
> +            length -= 4;
> +            return e.getInt();
> +        }
> +
> +        // Otherwise, read byte by byte. (inefficient!)
> +        int value = 0;
> +        for (int bytesToRead = 4; bytesToRead > 0; bytesToRead --) {
> +            value = value << 8 | e.get();
> +            length --;
> +            remaining --;
> +            if (remaining == 0) {
> +                e = safeElement();
> +                remaining = e.remaining();
> +            }
> +        }
> +
> +        return applyByteOrder(value);
> +    }
> +
> +    public long   removeLong() {
> +        checkSequentialAccess(8);
> +        // Try to read in one shot.
> +        ByteBuffer e = safeElement();
> +        int remaining = e.remaining();
> +        switch (remaining) {
> +        case 0: case 1: case 2: case 3: case 4: case 5: case 6: case 7:
> +            break;
> +        case 8:
> +            length -= 8;
> +            long value = e.getLong();
> +            trim();
> +            return value;
> +        default:
> +            length -= 8;
> +            return e.getLong();
> +        }
> +
> +        // Otherwise, read byte by byte. (inefficient!)
> +        long value = 0;
> +        for (int bytesToRead = 8; bytesToRead > 0; bytesToRead --) {
> +            value = value << 8 | e.get();
> +            length --;
> +            remaining --;
> +            if (remaining == 0) {
> +                e = safeElement();
> +                remaining = e.remaining();
> +            }
> +        }
> +
> +        return applyByteOrder(value);
> +    }
> +
> +    public float  removeFloat() {
> +        return Float.intBitsToFloat(removeInt());
> +    }
> +
> +    public double removeDouble() {
> +        return Double.longBitsToDouble(removeLong());
> +    }
> +
> +    public void   discard(int length) {
> +        checkSequentialAccess(length);
> +
> +        int bytesToDiscard = length;
> +        while (bytesToDiscard > 0) {
> +            ByteBuffer element = queue.peek();
> +            int remaining = element.remaining();
> +            if (remaining == 0) {
> +                removeAndAssert(element);
> +                continue;
> +            }
> +
> +            if (remaining >= bytesToDiscard) {
> +                element.position(element.position() + bytesToDiscard);
> +                this.length -= bytesToDiscard;
> +                break;
> +            }
> +
> +            removeAndAssert(element);
> +            element.limit(element.limit());
> +            bytesToDiscard -= remaining;
> +            this.length -= remaining;
> +        }
> +    }
> +
> +    public byte   elementAsByte  () {
> +        checkSequentialAccess(1);
> +        ByteBuffer e = safeElement();
> +        return e.get(e.position());
> +    }
> +
> +    public short  elementAsShort () {
> +        checkSequentialAccess(2);
> +        // Try to read in one shot.
> +        ByteBuffer e = safeElement();
> +        if (e.remaining() >= 2) {
> +            return e.getShort(e.position());
> +        }
> +
> +        // Otherwise, read byte by byte. (inefficient!)
> +        return applyByteOrder((short) readByteByByte(2));
> +    }
> +
> +    public int    elementAsInt   () {
> +        checkSequentialAccess(4);
> +        // Try to read in one shot.
> +        ByteBuffer e = safeElement();
> +        if (e.remaining() >= 4) {
> +            return e.getInt(e.position());
> +        }
> +
> +        // Otherwise, read byte by byte. (inefficient!)
> +        return applyByteOrder((int) readByteByByte(4));
> +    }
> +
> +    public long   elementAsLong  () {
> +        checkSequentialAccess(8);
> +        // Try to read in one shot.
> +        ByteBuffer e = safeElement();
> +        if (e.remaining() >= 8) {
> +            return e.getLong(e.position());
> +        }
> +
> +        // Otherwise, read byte by byte. (inefficient!)
> +        return applyByteOrder(readByteByByte(8));
> +    }
> +
> +    public float  elementAsFloat () {
> +        return Float.intBitsToFloat(elementAsInt());
> +    }
> +
> +    public double elementAsDouble() {
> +        return Double.longBitsToDouble(elementAsLong());
> +    }
> +
> +    public byte   getByte  (int byteIndex) {
> +        checkRandomAccess(byteIndex, 1);
> +        if (byteIndex == 0) {
> +            return elementAsByte();
> +        }
> +
> +        // Get the value from the first element if possible.
> +        ByteBuffer e = safeElement();
> +        if (e.remaining() > byteIndex) {
> +            return e.get(e.position() + byteIndex);
> +        }
> +
> +        // Otherwise, start expensive traversal.
> +        for (ByteBuffer b: this) {
> +            if (b.remaining() > byteIndex) {
> +                return e.get(e.position() + byteIndex);
> +            } else {
> +                byteIndex -= e.remaining();
> +            }
> +        }
> +
> +        // Should never reach here unless concurrent modification happened.
> +        throw new ConcurrentModificationException();
> +    }
> +
> +    public short  getShort (int byteIndex) {
> +        checkRandomAccess(byteIndex, 2);
> +        if (byteIndex == 0) {
> +            return elementAsByte();
> +        }
> +
> +        // Get the value from the first element if possible.
> +        ByteBuffer e = safeElement();
> +        if (e.remaining() >= byteIndex + 2) {
> +            return e.getShort(e.position() + byteIndex);
> +        }
> +
> +        // Otherwise, start expensive traversal.
> +        return applyByteOrder((short) getByteByByte(byteIndex, 2));
> +    }
> +
> +    public int    getInt   (int byteIndex) {
> +        checkRandomAccess(byteIndex, 4);
> +        if (byteIndex == 0) {
> +            return elementAsByte();
> +        }
> +
> +        // Get the value from the first element if possible.
> +        ByteBuffer e = safeElement();
> +        if (e.remaining() >= byteIndex + 4) {
> +            return e.getInt(e.position() + byteIndex);
> +        }
> +
> +        // Otherwise, start expensive traversal.
> +        return applyByteOrder((int) getByteByByte(byteIndex, 4));
> +    }
> +
> +    public long   getLong  (int byteIndex) {
> +        checkRandomAccess(byteIndex, 8);
> +        if (byteIndex == 0) {
> +            return elementAsByte();
> +        }
> +
> +        // Get the value from the first element if possible.
> +        ByteBuffer e = safeElement();
> +        if (e.remaining() >= byteIndex + 8) {
> +            return e.getLong(e.position() + byteIndex);
> +        }
> +
> +        // Otherwise, start expensive traversal.
> +        return applyByteOrder(getByteByByte(byteIndex, 8));
> +    }
> +
> +    public float  getFloat (int byteIndex) {
> +        return Float.intBitsToFloat(getInt(byteIndex));
> +    }
> +
> +    public double getDouble(int byteIndex) {
> +        return Double.longBitsToDouble(getLong(byteIndex));
> +    }
> +
> +    public ByteBuffer merge() {
> +        ByteBuffer buf = bufferFactory.newByteBuffer(length);
> +        buf.order(order);
> +        for (ByteBuffer e: queue) {
> +            buf.put(e.duplicate());
> +        }
> +        buf.clear();
> +        return buf;
> +    }
> +
> +    private ByteBuffer tail(int length) {
> +        ByteBuffer oldTail = tail;
> +        if (oldTail == null || oldTail.capacity() - oldTail.limit() < length) {
> +            ByteBuffer newTail = bufferFactory.newByteBuffer(capacityIncrement);
> +            newTail.order(order);
> +            newTail.limit(0);
> +            if (!queue.offer(newTail)) {
> +                return null;
> +            }
> +            tail = newTail;
> +            return newTail;
> +        } else {
> +            return oldTail;
> +        }
> +    }
> +
> +    /**
> +     * The same operation with {@link #element()} except that it never returns
> +     * an empty buffer.
> +     */
> +    private ByteBuffer safeElement() {
> +        for (;;) {
> +            ByteBuffer e = queue.element();
> +            if (e.hasRemaining()) {
> +                return e;
> +            } else {
> +                removeAndAssert(e);
> +            }
> +        }
> +    }
> +
> +    /**
> +     * Removes any empty buffers in the head of this queue.
> +     */
> +    private void trim() {
> +        for (;;) {
> +            ByteBuffer e = queue.peek();
> +            if (e == null || e.hasRemaining()) {
> +                break;
> +            }
> +            removeAndAssert(e);
> +        }
> +    }
> +
> +    /**
> +     * Removes the first element and make sure it is same with the specified
> +     * buffer.
> +     */
> +    private void removeAndAssert(ByteBuffer e) {
> +        ByteBuffer removedElement = queue.remove();
> +        assert removedElement == e;
> +    }
> +
> +    /**
> +     * Throws an exception if the specified length is illegal or the length of
> +     * this queue is less than the specified integer.
> +     */
> +    private void checkSequentialAccess(int length) {
> +        if (length < 0) {
> +            throw new IllegalArgumentException(
> +                    "length: " + length +
> +                    " (expected: zero or a positive integer)");
> +        }
> +
> +        if (this.length < length) {
> +            throw new NoSuchElementException();
> +        }
> +    }
> +
> +    /**
> +     * Throws an exception if the byteIndex is incorrect or the length of this
> +     * queue is less than the specified integer + byteIndex.
> +     */
> +    private void checkRandomAccess(int byteIndex, int length) {
> +        if (byteIndex < 0) {
> +            throw new IllegalArgumentException(
> +                    "byteIndex: " + byteIndex +
> +                    " (expected: 0 or a positive integer)");
> +        }
> +
> +        if (this.length < byteIndex + length) {
> +            throw new IndexOutOfBoundsException();
> +        }
> +    }
> +
> +    private long readByteByByte(int bytesToRead) {
> +        long value = 0;
> +        for (ByteBuffer b: this) {
> +            int remaining = b.remaining();
> +            for (int i = 0; i < remaining; i ++) {
> +                value = value << 8 | b.get(b.position() + i);
> +                bytesToRead --;
> +                if (bytesToRead == 0) {
> +                    return value;
> +                }
> +            }
> +        }
> +
> +        throw new ConcurrentModificationException();
> +    }
> +
> +    private long getByteByByte(int byteIndex, int bytesToRead) {
> +        long value = 0;
> +        for (ByteBuffer b: this) {
> +            int remaining = b.remaining();
> +
> +            // Skip until we find the element which matches to the offset.
> +            if (remaining <= byteIndex) {
> +                byteIndex -= remaining;
> +                continue;
> +            }
> +
> +            for (int i = 0; i < remaining; i ++) {
> +                value = value << 8 | b.get(b.position() + byteIndex);
> +                bytesToRead --;
> +                if (bytesToRead == 0) {
> +                    return value;
> +                }
> +                byteIndex ++;
> +            }
> +            byteIndex -= remaining;
> +        }
> +        throw new ConcurrentModificationException();
> +    }
> +
> +    private short applyByteOrder(short value) {
> +        // Reverse the bytes if necessary.
> +        if (order == ByteOrder.LITTLE_ENDIAN) {
> +            int newValue = value >>> 8 & 0xFF | (value & 0xFF) << 8;
> +            value = (short) newValue;
> +        }
> +        return value;
> +    }
> +
> +    private int applyByteOrder(int value) {
> +        // Reverse the bytes if necessary.
> +        if (order == ByteOrder.LITTLE_ENDIAN) {
> +            value = (value >>> 24 & 0xFF) <<  0 |
> +                    (value >>> 16 & 0xFF) <<  8 |
> +                    (value >>>  8 & 0xFF) << 16 |
> +                    (value >>>  0 & 0xFF) << 24;
> +        }
> +        return value;
> +    }
> +
> +    private long applyByteOrder(long value) {
> +        // Reverse the bytes if necessary.
> +        if (order == ByteOrder.LITTLE_ENDIAN) {
> +            value = (value >>> 56 & 0xFFL) <<  0 |
> +                    (value >>> 48 & 0xFFL) <<  8 |
> +                    (value >>> 40 & 0xFFL) << 16 |
> +                    (value >>> 32 & 0xFFL) << 24 |
> +                    (value >>> 24 & 0xFFL) << 32 |
> +                    (value >>> 16 & 0xFFL) << 40 |
> +                    (value >>>  8 & 0xFFL) << 48 |
> +                    (value >>>  0 & 0xFFL) << 56;
> +        }
> +        return value;
> +    }
> +}
>
> Propchange: mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java
> ------------------------------------------------------------------------------
>     svn:eol-style = native
>
> Propchange: mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java
> ------------------------------------------------------------------------------
>     svn:keywords = Rev Date
>
>
>
>   


-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
이희승 (Trustin Lee) wrote:
> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny 
> <el...@apache.org> wrote:
>
>> 이희승 (Trustin Lee) wrote:
>>> Every public methods except for the constructors are overridden from 
>>> its supertypes and interfaces.  They all got proper JavaDoc 
>>> comments.  Let me know if I am missing something.
>>
>> Adding a @see Class#method() in the implementation then should help. 
>> When you look at a method javadoc it's better to know where too look 
>> at : the intheritance scheme can be feilry complex, and it can be a 
>> burden to retreive the associated Javadoc.
>>
>> Something like :
>>     /**
>>      * @see javax.naming.Context#close()
>>      */
>>     public void close() throws NamingException
>> ...
>
> I'd just move the cursor on the method?  That shows pretty nicely 
> rendered JavaDoc in modern IDEs.  If you are interested in the class 
> hierarchy, press F4 in Eclipse.  It's what IDEs are supposed to do and 
> they already do so.
>
I would also add that @see is really the bare minimum. It would be MUCH 
better to copy/past the javadoc from the interface. Just have a look at 
List and ArrayList, to check how it's done in Java code.

Note that I would understand that until the code is finished, copying a 
bad javadoc is painfull, thus using the @see is a cool shortcut.

Thanks.

-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Julien Vermillard <jv...@archean.fr>.
On Fri, 23 May 2008 12:48:23 +0900
이희승 (Trustin Lee) <tr...@gmail.com> wrote:

> On Fri, 23 May 2008 12:44:00 +0900, Emmanuel Lecharny  
> <el...@apache.org> wrote:
> 
> > 이희승 (Trustin Lee) wrote:
> >> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny  
> >> <el...@apache.org> wrote:
> >>
> >>> 이희승 (Trustin Lee) wrote:
> >>>> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny  
> >>>> <el...@apache.org> wrote:
> >>>>
> >>>>> 이희승 (Trustin Lee) wrote:
> >>>>>> Every public methods except for the constructors are
> >>>>>> overridden from its supertypes and interfaces.  They all got
> >>>>>> proper JavaDoc comments.  Let me know if I am missing
> >>>>>> something.
> >>>>>
> >>>>> Adding a @see Class#method() in the implementation then should
> >>>>> help. When you look at a method javadoc it's better to know
> >>>>> where too look at : the intheritance scheme can be feilry
> >>>>> complex, and it can be a burden to retreive the associated
> >>>>> Javadoc.
> >>>>>
> >>>>> Something like :
> >>>>>     /**
> >>>>>      * @see javax.naming.Context#close()
> >>>>>      */
> >>>>>     public void close() throws NamingException
> >>>>> ...
> >>>>
> >>>> I'd just move the cursor on the method?  That shows pretty
> >>>> nicely rendered JavaDoc in modern IDEs.
> >>> Sometime, you just have to use vi or emacs. Make it simple for
> >>> users : add a @see tag. Cost almost nothing, and it helps.
> >>
> >> I wouldn't bother with vi or emacs.  They pay for what they use.   
> >> Moreover, it's not 'almost nothing'.
> > -1.
> >
> > Please revert the commit.
> 
> -1.  I have a valid point not to add those @see tags.
> 
> If you really want to see them added, add them by yourself.  I
> disagree with what you suggest anyway.
> 
> Now you are behaving like a manager, who is a bladder but does no
> actual action.
> 

Due to the number of mail Emmanuel is sending, I can't say he's doing
nothing here.

Julien

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Michael Qi <fo...@gmail.com>.
Hi Trustin,

Just a little question:
 Could  many SocketConnector share the same ThreadPoolExecutor?

Thank you
QiHe

On Fri, May 23, 2008 at 3:39 PM, 이희승 (Trustin Lee) <tr...@gmail.com> wrote:
> On Fri, 23 May 2008 14:32:13 +0900, Emmanuel Lecharny <el...@apache.org>
> wrote:
>
>> 이희승 (Trustin Lee) wrote:
>>>
>>> On Fri, 23 May 2008 12:44:00 +0900, Emmanuel Lecharny
>>> <el...@apache.org> wrote:
>>>
>>>> 이희승 (Trustin Lee) wrote:
>>>>>
>>>>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny
>>>>> <el...@apache.org> wrote:
>>>>>
>>>>>> 이희승 (Trustin Lee) wrote:
>>>>>>>
>>>>>>> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny
>>>>>>> <el...@apache.org> wrote:
>>>>>>>
>>>>>>>> 이희승 (Trustin Lee) wrote:
>>>>>>>>>
>>>>>>>>> Every public methods except for the constructors are overridden
>>>>>>>>> from its supertypes and interfaces.  They all got proper JavaDoc comments.
>>>>>>>>>  Let me know if I am missing something.
>>>>>>>>
>>>>>>>> Adding a @see Class#method() in the implementation then should help.
>>>>>>>> When you look at a method javadoc it's better to know where too look at :
>>>>>>>> the intheritance scheme can be feilry complex, and it can be a burden to
>>>>>>>> retreive the associated Javadoc.
>>>>>>>>
>>>>>>>> Something like :
>>>>>>>>    /**
>>>>>>>>     * @see javax.naming.Context#close()
>>>>>>>>     */
>>>>>>>>    public void close() throws NamingException
>>>>>>>> ...
>>>>>>>
>>>>>>> I'd just move the cursor on the method?  That shows pretty nicely
>>>>>>> rendered JavaDoc in modern IDEs.
>>>>>>
>>>>>> Sometime, you just have to use vi or emacs. Make it simple for users :
>>>>>> add a @see tag. Cost almost nothing, and it helps.
>>>>>
>>>>> I wouldn't bother with vi or emacs.  They pay for what they use.
>>>>>  Moreover, it's not 'almost nothing'.
>>>>
>>>> -1.
>>>>
>>>> Please revert the commit.
>>>
>>> -1.  I have a valid point not to add those @see tags.
>>>
>>> If you really want to see them added, add them by yourself.  I disagree
>>> with what you suggest anyway.
>>>
>>> Now you are behaving like a manager, who is a bladder but does no actual
>>> action.
>>>
>> Trustin, I'm on the project *management* committee.
>
> I know.  Why would I say that then?
>
>> I would like to see the code you commit adheres to some standard.
>>
>> Until this is resolved my right to veto holds so please revert your commit
>> until we figure out the best choice for Javadoc. If you won't revert it, I
>> can do it for you.
>
> What's apparent is that:
>
>  * @see is not appropriate for this case.
>  * Duplication is bad and it costs a lot of maintenance burden for us.
>
> for GOOD reason.
>
>> Regardless of which we choose something is required to help those using
>> IDE's besides Eclipse. There is no reason why Vi or Emacs users should
>> according to your words, 'pay for what they use'.
>
> You should say IDEs besides Eclipse, NetBeans and IntelliJ.  Then... what's
> left?  Vi and Emacs?  They just chose the wrong tool to navigate Java source
> code.
>
>> I also have to mention that I don't like to go that far. I have to just in
>> order to get this project on rails. You have to understand that you can't
>> break all the rules simply because you think that the rules are bad.
>> I didn't made the rules, and I consider that good practices are also rules
>> we have to follow. If all the implemented classes in the core JVM (ArrayList
>> and List, etc) duplicate the Javadoc, it's for a pretty good reason.
>> You can disagree, but that won't make those good reasons disappear.
>
> They didn't duplicate JavaDoc but rewrote or amended the original JavaDoc to
> clarify the implementation detail or performance characterstics.  If you
> look into more classes more in detail, you will even see empty JavaDoc
> blocks.
>
> Your good reason is not always others' good reason.  You already saw
> disagreement in using @see tags and duplicating JavaDoc.
>
> It simply doesn't make a sense to revert back a chunk of working
> implementation because of subtle documentation issue (I am even suspicious
> that it's really an issue).
>
>> PS : for the record, here is the veto definition
>> (http://www.apache.org/foundation/glossary.html) :
>>
>> *Veto*
>>    According to the Apache methodology, a change which has been made or
>>    proposed may be made moot through the exercise of a veto by a
>>    committer to the codebase
>>    <http://www.apache.org/foundation/glossary.html#Codebase> in
>>    question. If the R-T-C
>>    <http://www.apache.org/foundation/glossary.html#ReviewThenCommit>
>>    commit policy is in effect, a veto prevents the change from being
>>    made. In either the R-T-C or C-T-R
>>    <http://www.apache.org/foundation/glossary.html#CommitThenReview>
>>    environments, a veto applied to a change that has already been made
>>    forces it to be reverted. Vetos may not be overridden nor voted
>>    down, and only cease to apply when the committer who issued the veto
>>    withdraws it. All vetos /must/ be accompanied by a valid technical
>>    justification; a veto without such a justification is invalid. Vetos
>>    only apply to code changes; they do not apply to procedural issues
>>    such as software releases.
>
> Therefore, your veto is invalid.
>
> --
> Trustin Lee - Principal Software Engineer, JBoss, Red Hat
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
>

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Alex Karasulu <ak...@apache.org>.
On Fri, May 23, 2008 at 3:39 AM, 이희승 (Trustin Lee) <tr...@gmail.com>
wrote:

> On Fri, 23 May 2008 14:32:13 +0900, Emmanuel Lecharny <
> elecharny@apache.org> wrote:
>
>  이희승 (Trustin Lee) wrote:
>>
>>> On Fri, 23 May 2008 12:44:00 +0900, Emmanuel Lecharny <
>>> elecharny@apache.org> wrote:
>>>
>>>  이희승 (Trustin Lee) wrote:
>>>>
>>>>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny <
>>>>> elecharny@apache.org> wrote:
>>>>>
>>>>>  이희승 (Trustin Lee) wrote:
>>>>>>
>>>>>>> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny <
>>>>>>> elecharny@apache.org> wrote:
>>>>>>>
>>>>>>>  이희승 (Trustin Lee) wrote:
>>>>>>>>
>>>>>>>>> Every public methods except for the constructors are overridden
>>>>>>>>> from its supertypes and interfaces.  They all got proper JavaDoc comments.
>>>>>>>>>  Let me know if I am missing something.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Adding a @see Class#method() in the implementation then should help.
>>>>>>>> When you look at a method javadoc it's better to know where too look at :
>>>>>>>> the intheritance scheme can be feilry complex, and it can be a burden to
>>>>>>>> retreive the associated Javadoc.
>>>>>>>>
>>>>>>>> Something like :
>>>>>>>>    /**
>>>>>>>>     * @see javax.naming.Context#close()
>>>>>>>>     */
>>>>>>>>    public void close() throws NamingException
>>>>>>>> ...
>>>>>>>>
>>>>>>>
>>>>>>> I'd just move the cursor on the method?  That shows pretty nicely
>>>>>>> rendered JavaDoc in modern IDEs.
>>>>>>>
>>>>>> Sometime, you just have to use vi or emacs. Make it simple for users :
>>>>>> add a @see tag. Cost almost nothing, and it helps.
>>>>>>
>>>>>
>>>>> I wouldn't bother with vi or emacs.  They pay for what they use.
>>>>>  Moreover, it's not 'almost nothing'.
>>>>>
>>>> -1.
>>>>
>>>> Please revert the commit.
>>>>
>>>
>>> -1.  I have a valid point not to add those @see tags.
>>>
>>> If you really want to see them added, add them by yourself.  I disagree
>>> with what you suggest anyway.
>>>
>>> Now you are behaving like a manager, who is a bladder but does no actual
>>> action.
>>>
>>>  Trustin, I'm on the project *management* committee.
>>
>
> I know.  Why would I say that then?
>
>  I would like to see the code you commit adheres to some standard.
>>
>> Until this is resolved my right to veto holds so please revert your commit
>> until we figure out the best choice for Javadoc. If you won't revert it, I
>> can do it for you.
>>
>
> What's apparent is that:
>
>  * @see is not appropriate for this case.
>  * Duplication is bad and it costs a lot of maintenance burden for us.
>
> for GOOD reason.
>
>  Regardless of which we choose something is required to help those using
>> IDE's besides Eclipse. There is no reason why Vi or Emacs users should
>> according to your words, 'pay for what they use'.
>>
>
> You should say IDEs besides Eclipse, NetBeans and IntelliJ.  Then... what's
> left?  Vi and Emacs?  They just chose the wrong tool to navigate Java source
> code.
>
>  I also have to mention that I don't like to go that far. I have to just in
>> order to get this project on rails. You have to understand that you can't
>> break all the rules simply because you think that the rules are bad.
>> I didn't made the rules, and I consider that good practices are also rules
>> we have to follow. If all the implemented classes in the core JVM (ArrayList
>> and List, etc) duplicate the Javadoc, it's for a pretty good reason.
>> You can disagree, but that won't make those good reasons disappear.
>>
>
>
>  PS : for the record, here is the veto definition (
>> http://www.apache.org/foundation/glossary.html) :
>>
>> *Veto*
>>    According to the Apache methodology, a change which has been made or
>>    proposed may be made moot through the exercise of a veto by a
>>    committer to the codebase
>>    <http://www.apache.org/foundation/glossary.html#Codebase> in
>>    question. If the R-T-C
>>    <http://www.apache.org/foundation/glossary.html#ReviewThenCommit>
>>    commit policy is in effect, a veto prevents the change from being
>>    made. In either the R-T-C or C-T-R
>>    <http://www.apache.org/foundation/glossary.html#CommitThenReview>
>>    environments, a veto applied to a change that has already been made
>>    forces it to be reverted. Vetos may not be overridden nor voted
>>    down, and only cease to apply when the committer who issued the veto
>>    withdraws it. All vetos /must/ be accompanied by a valid technical
>>    justification; a veto without such a justification is invalid. Vetos
>>    only apply to code changes; they do not apply to procedural issues
>>    such as software releases.
>>
>
> Therefore, your veto is invalid.


Sorry that's not the case and you need to respect Emmanuel's veto.  You
cannot sit there discussing this ad nausium and brushing aside his concerns
which I describe below.  Otherwise you make this PMC utterly useless.  Let's
not make it into a bunch of powerless Trustin cheerleaders.

I know this whole Javadoc thing sounds rediculous but Mark Webb said well
that we're loosing the real point being made by Emmanuel.  You're committing
code without Javadocs quite often.  This is making it harder to get more
people into the core. In the old days of Avalon a veto was once cast over a
Javadoc change and it had to be respected.  Those who faught it weakened the
ability of the TLP to survive.

This could have been a descent conversation, but once again was degraded
into a pissing match.  Could we be a little less obstinate with one another
over these relatively non-issues.  Both you guys have some points.

Trustin it also does not help reeming people about their "lack of action".
Emmanuel has taken action and is involved in our community addressing
various matters.  It's not all about the code although he's in there trying
to get familiar.  At Avalon, the biggest problem was that some people
thought it was only about the code.  Could you please stop biting folks like
this?

Your going to start getting more vetos until you take absolutely every
precaution mandated to make the code as easy to pick up by others as
possible.  That's to protect the project and not to attack you.  Please
understand this.

Alex

P.S. I thought we had a great converstation over skype and it was clear how
and why there were issues but  the same thing is occuring again
systemically: maybe we need to have another private session?

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/Default ByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
Frédéric Brégier wrote:
> I didn't want to get this thread continues,
> but I made 7th tries to create a new thread
> and it was always blocked by the anti-spam
> apache mailer rules. I don't know why ?
>
> I had writing a personnal mail to say how
> I feel like an end user on this mailing list.
> I adore this Mina project, found very interesting
> what I read from it, except that several
> times I become sad to see those kind
> of riot without necessity.
> I don't want to blame anyone, since I
> perfectly understand the points of all.
> So don't take it personnaly, don't blame
> me but just take it as a external point of view...
>
> I know there must be rules, but could it be
> possible to get such discussion outside when it
> becomes difficult ?
>  PS: I really truncate my mail since I don't know
> which part is considered as a spam...
>
> Frederic
>
>   
Hi Frederic,

don't worry too much. Such a 'riot' happens from time to time, and it 
does not matter that much. People just have opinions, and expressing 
them through a ML is proven to be the wrong place : sometime it 
degenerates (like it did) simply because the asyncronous nature of a 
mail makes it frustrating.

Such a discussion in a F2F meeting would _not_ degenerate like this, and 
sometime a quick skype session simply cool down things (I have 
experimented it more than once !).

It's just a matter of time, and things settle by themselves. Sometime it 
also helps if people realize how futile was the conversation, and accept 
the idea that words are just words.

About the rules, as Talleyrand said : "Les règles, quand on les violent, 
ne se mettent pas à hurler" (basically, "rules don't yell when you break 
them").  People do.

So, to end this thread, I would like to personnally apologize if my 
mails have been too far, It was not my intention to hurt anyone, and my 
last mail was just a little bit too harsh. Anyway, forget about the 
messenger, and get the message : Javadoc is good to have.


-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/Default ByteBufferQueue.java

Posted by Frédéric Brégier <fr...@free.fr>.
I didn't want to get this thread continues,
but I made 7th tries to create a new thread
and it was always blocked by the anti-spam
apache mailer rules. I don't know why ?

I had writing a personnal mail to say how
I feel like an end user on this mailing list.
I adore this Mina project, found very interesting
what I read from it, except that several
times I become sad to see those kind
of riot without necessity.
I don't want to blame anyone, since I
perfectly understand the points of all.
So don't take it personnaly, don't blame
me but just take it as a external point of view...

I know there must be rules, but could it be
possible to get such discussion outside when it
becomes difficult ?
 PS: I really truncate my mail since I don't know
which part is considered as a spam...

Frederic

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/Default ByteBufferQueue.java

Posted by jv...@archean.fr.
> On Fri, May 23, 2008 at 1:04 PM, Stefano Bagnara <ap...@bago.org> wrote:
>
>> Emmanuel Lecharny ha scritto:
>>
>>> Now, considering your attitude : your are sometime just behaving like a
>>> stubborn childish person. You are interpreting the ASF rules at your
>>> own
>>> advantage, with no respect for person around you. Like it or not, this
>>> is
>>> not the way it works. When I asked you to revert the code, this is
>>> because
>>> we reached a point where we have to discuss the portion of code you
>>> have
>>> committed, it was not a punishment. But you took it as if I wanted to
>>> personally insult you, and you insulted me in return. I will swallow
>>> the
>>> insult, because it's just totally irrelevant to the discussion.
>>>
>>
>> My opinion is that you behaved very bad too. There was a discussion in
>> place, and there was no need to veto this until every solution was
>> evaluated. You didn't try to understand the why, propose something
>> different, evaluate pros/cons of what have been done and what you
>> proposed
>> before vetoing it.
>>
>> This is only my opinion as an ASF committer have only a user role in
>> mina.
>>
>
> No one asked for your precious opinion.  You don't have a grasp of the
> situation at all.  So stop adding to the fire.
>
> LET THE THREAD DIE!
>
> We need to move on.
>
> Alex
>
Hi,

Until it's on a public mailing list I don't see the problem with everybody
expressing his opinion, it's the purporse.

Julien


Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Mark Webb <el...@gmail.com>.
since when do people on mailing lists no longer get to voice opinions?
 What then would be the purpose of a mailing list?


On Fri, May 23, 2008 at 1:14 PM, Alex Karasulu <ak...@apache.org> wrote:
> On Fri, May 23, 2008 at 1:04 PM, Stefano Bagnara <ap...@bago.org> wrote:
>
>> Emmanuel Lecharny ha scritto:
>>
>>> Now, considering your attitude : your are sometime just behaving like a
>>> stubborn childish person. You are interpreting the ASF rules at your own
>>> advantage, with no respect for person around you. Like it or not, this is
>>> not the way it works. When I asked you to revert the code, this is because
>>> we reached a point where we have to discuss the portion of code you have
>>> committed, it was not a punishment. But you took it as if I wanted to
>>> personally insult you, and you insulted me in return. I will swallow the
>>> insult, because it's just totally irrelevant to the discussion.
>>>
>>
>> My opinion is that you behaved very bad too. There was a discussion in
>> place, and there was no need to veto this until every solution was
>> evaluated. You didn't try to understand the why, propose something
>> different, evaluate pros/cons of what have been done and what you proposed
>> before vetoing it.
>>
>> This is only my opinion as an ASF committer have only a user role in mina.
>>
>
> No one asked for your precious opinion.  You don't have a grasp of the
> situation at all.  So stop adding to the fire.
>
> LET THE THREAD DIE!
>
> We need to move on.
>
> Alex
>

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
Mark Webb wrote:
> heh.  how do we all feel now?  Pretty embarrassing....
>   
well, not that much. Bill Burke is a RedHat employee, former JBoss 
employee, and may express his opinion through his blog at will. I'm just 
disappointed that he just extracted a small part of the thread. Anyways...

In two days, all of this will be over.

-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Mark Webb <el...@gmail.com>.
heh.  how do we all feel now?  Pretty embarrassing....

http://bill.burkecentral.com/2008/05/23/see-why-id-never-develop-at-apache/



On Fri, May 23, 2008 at 1:40 PM, Emmanuel Lecharny <el...@apache.org> wrote:
> Stefano Bagnara wrote:
>>
>> Alex Karasulu ha scritto:
>>>
>>> On Fri, May 23, 2008 at 1:04 PM, Stefano Bagnara <ap...@bago.org> wrote:
>>>>
>>>> This is only my opinion as an ASF committer have only a user role in
>>>> mina.
>>>
>>> No one asked for your precious opinion.  You don't have a grasp of the
>>> situation at all.  So stop adding to the fire.
>>
>> This is very community oriented ;-)
>> I will tell another opinion from mine even if no one asked it. My opinion
>> is that ASF is about community, and I feel part of this community, and I
>> have the same rights to express my opinion as you do.
>>
>> Peace,
>> Stefano
>>
>>
> Guys,
>
> just consider that not agree with the other one does not mean you are right.
>
> I would suggest we start another thread about Javadoc good practice, if
> everyone thinks it's necessary, and cut a decision in this thread.
>
> I think that everything is set now, whatever opinion everyone has. I have
> nothing to add, Trustin added some @inheritdoc tags, I have withdrawn my
> veto, end of story.
>
> Let's this thread die...
>
> Thanks !
>
> --
> --
> cordialement, regards,
> Emmanuel Lécharny
> www.iktek.com
> directory.apache.org
>
>
>

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
Stefano Bagnara wrote:
> Alex Karasulu ha scritto:
>> On Fri, May 23, 2008 at 1:04 PM, Stefano Bagnara <ap...@bago.org> 
>> wrote:
>>> This is only my opinion as an ASF committer have only a user role in 
>>> mina.
>>
>> No one asked for your precious opinion.  You don't have a grasp of the
>> situation at all.  So stop adding to the fire.
>
> This is very community oriented ;-)
> I will tell another opinion from mine even if no one asked it. My 
> opinion is that ASF is about community, and I feel part of this 
> community, and I have the same rights to express my opinion as you do.
>
> Peace,
> Stefano
>
>
Guys,

just consider that not agree with the other one does not mean you are 
right.

I would suggest we start another thread about Javadoc good practice, if 
everyone thinks it's necessary, and cut a decision in this thread.

I think that everything is set now, whatever opinion everyone has. I 
have nothing to add, Trustin added some @inheritdoc tags, I have 
withdrawn my veto, end of story.

Let's this thread die...

Thanks !

-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Stefano Bagnara <ap...@bago.org>.
Alex Karasulu ha scritto:
> On Fri, May 23, 2008 at 1:04 PM, Stefano Bagnara <ap...@bago.org> wrote:
>> This is only my opinion as an ASF committer have only a user role in mina.
>
> No one asked for your precious opinion.  You don't have a grasp of the
> situation at all.  So stop adding to the fire.

This is very community oriented ;-)
I will tell another opinion from mine even if no one asked it. My 
opinion is that ASF is about community, and I feel part of this 
community, and I have the same rights to express my opinion as you do.

Peace,
Stefano


Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Alex Karasulu <ak...@apache.org>.
On Fri, May 23, 2008 at 1:04 PM, Stefano Bagnara <ap...@bago.org> wrote:

> Emmanuel Lecharny ha scritto:
>
>> Now, considering your attitude : your are sometime just behaving like a
>> stubborn childish person. You are interpreting the ASF rules at your own
>> advantage, with no respect for person around you. Like it or not, this is
>> not the way it works. When I asked you to revert the code, this is because
>> we reached a point where we have to discuss the portion of code you have
>> committed, it was not a punishment. But you took it as if I wanted to
>> personally insult you, and you insulted me in return. I will swallow the
>> insult, because it's just totally irrelevant to the discussion.
>>
>
> My opinion is that you behaved very bad too. There was a discussion in
> place, and there was no need to veto this until every solution was
> evaluated. You didn't try to understand the why, propose something
> different, evaluate pros/cons of what have been done and what you proposed
> before vetoing it.
>
> This is only my opinion as an ASF committer have only a user role in mina.
>

No one asked for your precious opinion.  You don't have a grasp of the
situation at all.  So stop adding to the fire.

LET THE THREAD DIE!

We need to move on.

Alex

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Stefano Bagnara <ap...@bago.org>.
Emmanuel Lecharny ha scritto:
> Now, considering your attitude : your are sometime just behaving like a 
> stubborn childish person. You are interpreting the ASF rules at your own 
> advantage, with no respect for person around you. Like it or not, this 
> is not the way it works. When I asked you to revert the code, this is 
> because we reached a point where we have to discuss the portion of code 
> you have committed, it was not a punishment. But you took it as if I 
> wanted to personally insult you, and you insulted me in return. I will 
> swallow the insult, because it's just totally irrelevant to the discussion.

My opinion is that you behaved very bad too. There was a discussion in 
place, and there was no need to veto this until every solution was 
evaluated. You didn't try to understand the why, propose something 
different, evaluate pros/cons of what have been done and what you 
proposed before vetoing it.

This is only my opinion as an ASF committer have only a user role in mina.

Stefano


Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
As pointed out by Mark Webb, the initial intention was to avoid having 
long classes without ANY javadoc.As a developper, I don't really care 
that a class implementing an interface also inherits the javadoc. I'd 
rather get some hint about which javadoc is inherited, by the explicit 
use of whatever @see or @inehritdoc tag. Otherwise, you have no clue if 
the class has been written by a lazy guy who don't care about users or 
by someone who consider that it's user's business to make the difference.

The fact that we are almost all using 'modern' IDE is totally irrelevant 
to the issue. Considering Eclipse, waiting half a second on top of a 
method declaration in order to get the small yellow box to open, then 
having to press F2 to get the full content of the javadoc, with sliders 
on right side and bottom is just a PITA. Having a very simple javadoc on 
the code is so much simpler, and cost energy to one simgle person 
instead of annoying hundreds. We are writing code for others, not for 
our own pleasure.

Now, considering your attitude : your are sometime just behaving like a 
stubborn childish person. You are interpreting the ASF rules at your own 
advantage, with no respect for person around you. Like it or not, this 
is not the way it works. When I asked you to revert the code, this is 
because we reached a point where we have to discuss the portion of code 
you have committed, it was not a punishment. But you took it as if I 
wanted to personally insult you, and you insulted me in return. I will 
swallow the insult, because it's just totally irrelevant to the discussion.

I don't care about what you think as a person, I care about what you do 
as a committer on an ASF project. This is what I want you to consider.

-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
On Fri, 23 May 2008 14:32:13 +0900, Emmanuel Lecharny  
<el...@apache.org> wrote:

> 이희승 (Trustin Lee) wrote:
>> On Fri, 23 May 2008 12:44:00 +0900, Emmanuel Lecharny  
>> <el...@apache.org> wrote:
>>
>>> 이희승 (Trustin Lee) wrote:
>>>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny  
>>>> <el...@apache.org> wrote:
>>>>
>>>>> 이희승 (Trustin Lee) wrote:
>>>>>> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny  
>>>>>> <el...@apache.org> wrote:
>>>>>>
>>>>>>> 이희승 (Trustin Lee) wrote:
>>>>>>>> Every public methods except for the constructors are overridden  
>>>>>>>> from its supertypes and interfaces.  They all got proper JavaDoc  
>>>>>>>> comments.  Let me know if I am missing something.
>>>>>>>
>>>>>>> Adding a @see Class#method() in the implementation then should  
>>>>>>> help. When you look at a method javadoc it's better to know where  
>>>>>>> too look at : the intheritance scheme can be feilry complex, and  
>>>>>>> it can be a burden to retreive the associated Javadoc.
>>>>>>>
>>>>>>> Something like :
>>>>>>>     /**
>>>>>>>      * @see javax.naming.Context#close()
>>>>>>>      */
>>>>>>>     public void close() throws NamingException
>>>>>>> ...
>>>>>>
>>>>>> I'd just move the cursor on the method?  That shows pretty nicely  
>>>>>> rendered JavaDoc in modern IDEs.
>>>>> Sometime, you just have to use vi or emacs. Make it simple for users  
>>>>> : add a @see tag. Cost almost nothing, and it helps.
>>>>
>>>> I wouldn't bother with vi or emacs.  They pay for what they use.   
>>>> Moreover, it's not 'almost nothing'.
>>> -1.
>>>
>>> Please revert the commit.
>>
>> -1.  I have a valid point not to add those @see tags.
>>
>> If you really want to see them added, add them by yourself.  I disagree  
>> with what you suggest anyway.
>>
>> Now you are behaving like a manager, who is a bladder but does no  
>> actual action.
>>
> Trustin, I'm on the project *management* committee.

I know.  Why would I say that then?

> I would like to see the code you commit adheres to some standard.
>
> Until this is resolved my right to veto holds so please revert your  
> commit until we figure out the best choice for Javadoc. If you won't  
> revert it, I can do it for you.

What's apparent is that:

  * @see is not appropriate for this case.
  * Duplication is bad and it costs a lot of maintenance burden for us.

for GOOD reason.

> Regardless of which we choose something is required to help those using  
> IDE's besides Eclipse. There is no reason why Vi or Emacs users should  
> according to your words, 'pay for what they use'.

You should say IDEs besides Eclipse, NetBeans and IntelliJ.  Then...  
what's left?  Vi and Emacs?  They just chose the wrong tool to navigate  
Java source code.

> I also have to mention that I don't like to go that far. I have to just  
> in order to get this project on rails. You have to understand that you  
> can't break all the rules simply because you think that the rules are  
> bad.
> I didn't made the rules, and I consider that good practices are also  
> rules we have to follow. If all the implemented classes in the core JVM  
> (ArrayList and List, etc) duplicate the Javadoc, it's for a pretty good  
> reason.
> You can disagree, but that won't make those good reasons disappear.

They didn't duplicate JavaDoc but rewrote or amended the original JavaDoc  
to clarify the implementation detail or performance characterstics.  If  
you look into more classes more in detail, you will even see empty JavaDoc  
blocks.

Your good reason is not always others' good reason.  You already saw  
disagreement in using @see tags and duplicating JavaDoc.

It simply doesn't make a sense to revert back a chunk of working  
implementation because of subtle documentation issue (I am even suspicious  
that it's really an issue).

> PS : for the record, here is the veto definition  
> (http://www.apache.org/foundation/glossary.html) :
>
> *Veto*
>     According to the Apache methodology, a change which has been made or
>     proposed may be made moot through the exercise of a veto by a
>     committer to the codebase
>     <http://www.apache.org/foundation/glossary.html#Codebase> in
>     question. If the R-T-C
>     <http://www.apache.org/foundation/glossary.html#ReviewThenCommit>
>     commit policy is in effect, a veto prevents the change from being
>     made. In either the R-T-C or C-T-R
>     <http://www.apache.org/foundation/glossary.html#CommitThenReview>
>     environments, a veto applied to a change that has already been made
>     forces it to be reverted. Vetos may not be overridden nor voted
>     down, and only cease to apply when the committer who issued the veto
>     withdraws it. All vetos /must/ be accompanied by a valid technical
>     justification; a veto without such a justification is invalid. Vetos
>     only apply to code changes; they do not apply to procedural issues
>     such as software releases.

Therefore, your veto is invalid.

-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
이희승 (Trustin Lee) wrote:
> On Fri, 23 May 2008 12:44:00 +0900, Emmanuel Lecharny 
> <el...@apache.org> wrote:
>
>> 이희승 (Trustin Lee) wrote:
>>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny 
>>> <el...@apache.org> wrote:
>>>
>>>> 이희승 (Trustin Lee) wrote:
>>>>> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny 
>>>>> <el...@apache.org> wrote:
>>>>>
>>>>>> 이희승 (Trustin Lee) wrote:
>>>>>>> Every public methods except for the constructors are overridden 
>>>>>>> from its supertypes and interfaces.  They all got proper JavaDoc 
>>>>>>> comments.  Let me know if I am missing something.
>>>>>>
>>>>>> Adding a @see Class#method() in the implementation then should 
>>>>>> help. When you look at a method javadoc it's better to know where 
>>>>>> too look at : the intheritance scheme can be feilry complex, and 
>>>>>> it can be a burden to retreive the associated Javadoc.
>>>>>>
>>>>>> Something like :
>>>>>>     /**
>>>>>>      * @see javax.naming.Context#close()
>>>>>>      */
>>>>>>     public void close() throws NamingException
>>>>>> ...
>>>>>
>>>>> I'd just move the cursor on the method?  That shows pretty nicely 
>>>>> rendered JavaDoc in modern IDEs.
>>>> Sometime, you just have to use vi or emacs. Make it simple for 
>>>> users : add a @see tag. Cost almost nothing, and it helps.
>>>
>>> I wouldn't bother with vi or emacs.  They pay for what they use.  
>>> Moreover, it's not 'almost nothing'.
>> -1.
>>
>> Please revert the commit.
>
> -1.  I have a valid point not to add those @see tags.
>
> If you really want to see them added, add them by yourself.  I 
> disagree with what you suggest anyway.
>
> Now you are behaving like a manager, who is a bladder but does no 
> actual action.
>
Trustin, I'm on the project *management* committee.

I would like to see the code you commit adheres to some standard.

Until this is resolved my right to veto holds so please revert your 
commit until we figure out the best choice for Javadoc. If you won't 
revert it, I can do it for you.

Regardless of which we choose something is required to help those using 
IDE's besides Eclipse. There is no reason why Vi or Emacs users should 
according to your words, 'pay for what they use'.

I also have to mention that I don't like to go that far. I have to just 
in order to get this project on rails. You have to understand that you 
can't break all the rules simply because you think that the rules are bad.
I didn't made the rules, and I consider that good practices are also 
rules we have to follow. If all the implemented classes in the core JVM 
(ArrayList and List, etc) duplicate the Javadoc, it's for a pretty good 
reason.
You can disagree, but that won't make those good reasons disappear.

PS : for the record, here is the veto definition 
(http://www.apache.org/foundation/glossary.html) :

*Veto*
    According to the Apache methodology, a change which has been made or
    proposed may be made moot through the exercise of a veto by a
    committer to the codebase
    <http://www.apache.org/foundation/glossary.html#Codebase> in
    question. If the R-T-C
    <http://www.apache.org/foundation/glossary.html#ReviewThenCommit>
    commit policy is in effect, a veto prevents the change from being
    made. In either the R-T-C or C-T-R
    <http://www.apache.org/foundation/glossary.html#CommitThenReview>
    environments, a veto applied to a change that has already been made
    forces it to be reverted. Vetos may not be overridden nor voted
    down, and only cease to apply when the committer who issued the veto
    withdraws it. All vetos /must/ be accompanied by a valid technical
    justification; a veto without such a justification is invalid. Vetos
    only apply to code changes; they do not apply to procedural issues
    such as software releases.



-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
On Fri, 23 May 2008 12:44:00 +0900, Emmanuel Lecharny  
<el...@apache.org> wrote:

> 이희승 (Trustin Lee) wrote:
>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny  
>> <el...@apache.org> wrote:
>>
>>> 이희승 (Trustin Lee) wrote:
>>>> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny  
>>>> <el...@apache.org> wrote:
>>>>
>>>>> 이희승 (Trustin Lee) wrote:
>>>>>> Every public methods except for the constructors are overridden  
>>>>>> from its supertypes and interfaces.  They all got proper JavaDoc  
>>>>>> comments.  Let me know if I am missing something.
>>>>>
>>>>> Adding a @see Class#method() in the implementation then should help.  
>>>>> When you look at a method javadoc it's better to know where too look  
>>>>> at : the intheritance scheme can be feilry complex, and it can be a  
>>>>> burden to retreive the associated Javadoc.
>>>>>
>>>>> Something like :
>>>>>     /**
>>>>>      * @see javax.naming.Context#close()
>>>>>      */
>>>>>     public void close() throws NamingException
>>>>> ...
>>>>
>>>> I'd just move the cursor on the method?  That shows pretty nicely  
>>>> rendered JavaDoc in modern IDEs.
>>> Sometime, you just have to use vi or emacs. Make it simple for users :  
>>> add a @see tag. Cost almost nothing, and it helps.
>>
>> I wouldn't bother with vi or emacs.  They pay for what they use.   
>> Moreover, it's not 'almost nothing'.
> -1.
>
> Please revert the commit.

-1.  I have a valid point not to add those @see tags.

If you really want to see them added, add them by yourself.  I disagree  
with what you suggest anyway.

Now you are behaving like a manager, who is a bladder but does no actual  
action.

-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Brad Harvey <ha...@gmail.com>.
Hi Mark,

I disagree on two counts:
1. The code does have documentation because it inherits it, and this
inherited documentation is viewable with widely available, widely used,
free tools.
2. Anything is not necessarily better than nothing. You can turn on
automatic comment generation in eclipse, but I would argue that this is
definitely worse than nothing - you end up with javadoc, but it doesn't
tell you anything that can't be implied from the code.

Having said that, I think what you were getting at (apologies for taking
liberties!) is that not everything can have perfect documentation but
that there should be some. And with this I agree, and think there are
some places which give greater bang for buck as far as usefulness of the
documentation goes, and some where the returns dwindle.

Cheers,
Brad.

Mark Webb wrote:
> As I try and follow this thread, I think we have lost the point to the
> original issue which is that code is being checked in with no
> javadocs.  Sure, getting the proper javadoc tags is ideal, but
> anything is better than nothing.
>
>
> On Fri, May 23, 2008 at 4:45 AM, Brad Harvey <ha...@gmail.com> wrote:
>   
>> Hi All,
>>
>> I'll add my opinion as a MINA user rather than developer.  It's very
>> important to make sure the javadoc for the public API is correct and
>> complete.  In this case that's the ByteBufferQueue interface.  For the
>> implementation class it's useful to have class level javadoc describing its
>> characteristics and/or why I might use it rather than some other
>> implementation (like the Collections javadocs), but a blow by blow on each
>> public method really isn't necessary.  Either its already covered by
>> interface docs, or I'm probably not going to see them method entirely
>> because I'm programming to the interface anyway.
>>
>> MINA maintainers probably want comments about implementation details in the
>> implementation classes, but these are generally not javadoc comments on
>> public methods anyway.
>>
>> Also remember the tools (IDE & javadoc) are only getting better with time.
>>  With primitive / non specialised tools you'll probably have more
>> difficulties to deal with than not having @see tags.
>>
>> Cheers,
>> Brad.
>>
>> Stefano Bagnara wrote:
>>     
>>> Emmanuel Lecharny ha scritto:
>>>       
>>>> 이희승 (Trustin Lee) wrote:
>>>>         
>>>>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny
>>>>>           
>>>>>> Sometime, you just have to use vi or emacs. Make it simple for users :
>>>>>> add a @see tag. Cost almost nothing, and it helps.
>>>>>>             
>>>>> I wouldn't bother with vi or emacs.  They pay for what they use.
>>>>>  Moreover, it's not 'almost nothing'.
>>>>>           
>>>> -1.
>>>>
>>>> Please revert the commit.
>>>>
>>>> Thanks.
>>>>         
>>> Emmanuel, are you vetoing the commit because an inherited class (having
>>> full javadocs for the superclass) does not have method comments?
>>>
>>> Isn't this a bit hard at this point of the discussion?
>>>
>>> FWIW (I'm not committer to mina, but only a developer) I agree with
>>> Trustin.
>>>
>>> If a @see is useful then the javadoc tools should automatically add the
>>> see to every implementation method having no javadoc. This is about javadoc
>>> tools, not documentation. The documentation is already in the java source
>>> keywords "extends" and "implements".
>>>
>>> Stefano
>>>
>>>
>>>       

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Mark Webb <el...@gmail.com>.
As I try and follow this thread, I think we have lost the point to the
original issue which is that code is being checked in with no
javadocs.  Sure, getting the proper javadoc tags is ideal, but
anything is better than nothing.


On Fri, May 23, 2008 at 4:45 AM, Brad Harvey <ha...@gmail.com> wrote:
> Hi All,
>
> I'll add my opinion as a MINA user rather than developer.  It's very
> important to make sure the javadoc for the public API is correct and
> complete.  In this case that's the ByteBufferQueue interface.  For the
> implementation class it's useful to have class level javadoc describing its
> characteristics and/or why I might use it rather than some other
> implementation (like the Collections javadocs), but a blow by blow on each
> public method really isn't necessary.  Either its already covered by
> interface docs, or I'm probably not going to see them method entirely
> because I'm programming to the interface anyway.
>
> MINA maintainers probably want comments about implementation details in the
> implementation classes, but these are generally not javadoc comments on
> public methods anyway.
>
> Also remember the tools (IDE & javadoc) are only getting better with time.
>  With primitive / non specialised tools you'll probably have more
> difficulties to deal with than not having @see tags.
>
> Cheers,
> Brad.
>
> Stefano Bagnara wrote:
>>
>> Emmanuel Lecharny ha scritto:
>>>
>>> 이희승 (Trustin Lee) wrote:
>>>>
>>>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny
>>>>>
>>>>> Sometime, you just have to use vi or emacs. Make it simple for users :
>>>>> add a @see tag. Cost almost nothing, and it helps.
>>>>
>>>> I wouldn't bother with vi or emacs.  They pay for what they use.
>>>>  Moreover, it's not 'almost nothing'.
>>>
>>> -1.
>>>
>>> Please revert the commit.
>>>
>>> Thanks.
>>
>> Emmanuel, are you vetoing the commit because an inherited class (having
>> full javadocs for the superclass) does not have method comments?
>>
>> Isn't this a bit hard at this point of the discussion?
>>
>> FWIW (I'm not committer to mina, but only a developer) I agree with
>> Trustin.
>>
>> If a @see is useful then the javadoc tools should automatically add the
>> see to every implementation method having no javadoc. This is about javadoc
>> tools, not documentation. The documentation is already in the java source
>> keywords "extends" and "implements".
>>
>> Stefano
>>
>>
>

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Brad Harvey <ha...@gmail.com>.
Hi All,

I'll add my opinion as a MINA user rather than developer.  It's very 
important to make sure the javadoc for the public API is correct and 
complete.  In this case that's the ByteBufferQueue interface.  For the 
implementation class it's useful to have class level javadoc describing 
its characteristics and/or why I might use it rather than some other 
implementation (like the Collections javadocs), but a blow by blow on 
each public method really isn't necessary.  Either its already covered 
by interface docs, or I'm probably not going to see them method entirely 
because I'm programming to the interface anyway.

MINA maintainers probably want comments about implementation details in 
the implementation classes, but these are generally not javadoc comments 
on public methods anyway.

Also remember the tools (IDE & javadoc) are only getting better with 
time.  With primitive / non specialised tools you'll probably have more 
difficulties to deal with than not having @see tags.

Cheers,
Brad.

Stefano Bagnara wrote:
> Emmanuel Lecharny ha scritto:
>> 이희승 (Trustin Lee) wrote:
>>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny
>>>> Sometime, you just have to use vi or emacs. Make it simple for 
>>>> users : add a @see tag. Cost almost nothing, and it helps.
>>>
>>> I wouldn't bother with vi or emacs.  They pay for what they use.  
>>> Moreover, it's not 'almost nothing'.
>> -1.
>>
>> Please revert the commit.
>>
>> Thanks.
>
> Emmanuel, are you vetoing the commit because an inherited class 
> (having full javadocs for the superclass) does not have method comments?
>
> Isn't this a bit hard at this point of the discussion?
>
> FWIW (I'm not committer to mina, but only a developer) I agree with 
> Trustin.
>
> If a @see is useful then the javadoc tools should automatically add 
> the see to every implementation method having no javadoc. This is 
> about javadoc tools, not documentation. The documentation is already 
> in the java source keywords "extends" and "implements".
>
> Stefano
>
>

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Stefano Bagnara <ap...@bago.org>.
Emmanuel Lecharny ha scritto:
> 이희승 (Trustin Lee) wrote:
>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny 
>>> Sometime, you just have to use vi or emacs. Make it simple for users 
>>> : add a @see tag. Cost almost nothing, and it helps.
>>
>> I wouldn't bother with vi or emacs.  They pay for what they use.  
>> Moreover, it's not 'almost nothing'.
> -1.
> 
> Please revert the commit.
> 
> Thanks.

Emmanuel, are you vetoing the commit because an inherited class (having 
full javadocs for the superclass) does not have method comments?

Isn't this a bit hard at this point of the discussion?

FWIW (I'm not committer to mina, but only a developer) I agree with Trustin.

If a @see is useful then the javadoc tools should automatically add the 
see to every implementation method having no javadoc. This is about 
javadoc tools, not documentation. The documentation is already in the 
java source keywords "extends" and "implements".

Stefano


Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
이희승 (Trustin Lee) wrote:
> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny 
> <el...@apache.org> wrote:
>
>> 이희승 (Trustin Lee) wrote:
>>> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny 
>>> <el...@apache.org> wrote:
>>>
>>>> 이희승 (Trustin Lee) wrote:
>>>>> Every public methods except for the constructors are overridden 
>>>>> from its supertypes and interfaces.  They all got proper JavaDoc 
>>>>> comments.  Let me know if I am missing something.
>>>>
>>>> Adding a @see Class#method() in the implementation then should 
>>>> help. When you look at a method javadoc it's better to know where 
>>>> too look at : the intheritance scheme can be feilry complex, and it 
>>>> can be a burden to retreive the associated Javadoc.
>>>>
>>>> Something like :
>>>>     /**
>>>>      * @see javax.naming.Context#close()
>>>>      */
>>>>     public void close() throws NamingException
>>>> ...
>>>
>>> I'd just move the cursor on the method?  That shows pretty nicely 
>>> rendered JavaDoc in modern IDEs.
>> Sometime, you just have to use vi or emacs. Make it simple for users 
>> : add a @see tag. Cost almost nothing, and it helps.
>
> I wouldn't bother with vi or emacs.  They pay for what they use.  
> Moreover, it's not 'almost nothing'.
-1.

Please revert the commit.

Thanks.


-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Julien Vermillard <jv...@archean.fr>.
On Fri, 23 May 2008 15:44:32 +0900
이희승 (Trustin Lee) <tr...@gmail.com> wrote:

> On Fri, 23 May 2008 15:38:01 +0900, Julien Vermillard  
> <jv...@archean.fr> wrote:
> 
> > On Fri, 23 May 2008 12:31:14 +0900
> > 이희승 (Trustin Lee) <tr...@gmail.com> wrote:
> >
> >> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny
> >> <el...@apache.org> wrote:
> >>
> >> > 이희승 (Trustin Lee) wrote:
> >> >> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny
> >> >> <el...@apache.org> wrote:
> >> >>
> >> >>> 이희승 (Trustin Lee) wrote:
> >> >>>> Every public methods except for the constructors are
> >> >>>> overridden from its supertypes and interfaces.  They all got
> >> >>>> proper JavaDoc comments.  Let me know if I am missing
> >> >>>> something.
> >> >>>
> >> >>> Adding a @see Class#method() in the implementation then should
> >> >>> help. When you look at a method javadoc it's better to know
> >> >>> where too look at : the intheritance scheme can be feilry
> >> >>> complex, and it can be a burden to retreive the associated
> >> >>> Javadoc.
> >> >>>
> >> >>> Something like :
> >> >>>     /**
> >> >>>      * @see javax.naming.Context#close()
> >> >>>      */
> >> >>>     public void close() throws NamingException
> >> >>> ...
> >> >>
> >> >> I'd just move the cursor on the method?  That shows pretty
> >> >> nicely rendered JavaDoc in modern IDEs.
> >> > Sometime, you just have to use vi or emacs. Make it simple for
> >> > users : add a @see tag. Cost almost nothing, and it helps.
> >>
> >> I wouldn't bother with vi or emacs.  They pay for what they use.
> >> Moreover, it's not 'almost nothing'.
> >>
> > Hi,
> >
> > Well I use vi sometimes and a @see or @inheritedDoc would help. I
> > agree with Emmanuel, I don't feel I need to pay something for using
> > vi ;)
> 
> @inheritDoc already has been added as suggested by Mike and Rich.
> Now could you let me know how @see helps your vi experience and how
> often you use vi to browse the source code seriously?  I *sometimes*
> even use viewvc to browse the source code and didn't get any help
> from @see. 
> 
> > And that would make html generated javadoc much readable.
> 
> You mean @see or @inheritDoc?  You already get the copy of
> supertype's documentation and the links to the supertypes without
> @see and @inheritDoc.
> 

I use vi for some old java project using oldish javadoc generation who
look like is not copying the inherited javadoc.

The generated javadoc from 1.5 for MINA looks correct, my bad.

Julien

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
On Fri, 23 May 2008 15:38:01 +0900, Julien Vermillard  
<jv...@archean.fr> wrote:

> On Fri, 23 May 2008 12:31:14 +0900
> 이희승 (Trustin Lee) <tr...@gmail.com> wrote:
>
>> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny
>> <el...@apache.org> wrote:
>>
>> > 이희승 (Trustin Lee) wrote:
>> >> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny
>> >> <el...@apache.org> wrote:
>> >>
>> >>> 이희승 (Trustin Lee) wrote:
>> >>>> Every public methods except for the constructors are overridden
>> >>>> from its supertypes and interfaces.  They all got proper
>> >>>> JavaDoc comments.  Let me know if I am missing something.
>> >>>
>> >>> Adding a @see Class#method() in the implementation then should
>> >>> help. When you look at a method javadoc it's better to know where
>> >>> too look at : the intheritance scheme can be feilry complex, and
>> >>> it can be a burden to retreive the associated Javadoc.
>> >>>
>> >>> Something like :
>> >>>     /**
>> >>>      * @see javax.naming.Context#close()
>> >>>      */
>> >>>     public void close() throws NamingException
>> >>> ...
>> >>
>> >> I'd just move the cursor on the method?  That shows pretty nicely
>> >> rendered JavaDoc in modern IDEs.
>> > Sometime, you just have to use vi or emacs. Make it simple for
>> > users : add a @see tag. Cost almost nothing, and it helps.
>>
>> I wouldn't bother with vi or emacs.  They pay for what they use.
>> Moreover, it's not 'almost nothing'.
>>
> Hi,
>
> Well I use vi sometimes and a @see or @inheritedDoc would help. I
> agree with Emmanuel, I don't feel I need to pay something for using
> vi ;)

@inheritDoc already has been added as suggested by Mike and Rich.  Now  
could you let me know how @see helps your vi experience and how often you  
use vi to browse the source code seriously?  I *sometimes* even use viewvc  
to browse the source code and didn't get any help from @see.

> And that would make html generated javadoc much readable.

You mean @see or @inheritDoc?  You already get the copy of supertype's  
documentation and the links to the supertypes without @see and @inheritDoc.

-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Julien Vermillard <jv...@archean.fr>.
On Fri, 23 May 2008 10:18:10 +0200
Stefano Bagnara <ap...@bago.org> wrote:

> Julien Vermillard ha scritto:
> > Well I use vi sometimes and a @see or @inheritedDoc would help. I
> > agree with Emmanuel, I don't feel I need to pay something for using
> > vi ;)
> > 
> > And that would make html generated javadoc much readable.
> 
> So you would also like to have the list of class implementing a given 
> interface in an interface javadoc, and a comment saying that a method 
> override a method from the superclass, and a comment saying what are
> the transitive dependencies for the given class, and so on....
> 
> Why don't you simply use vi in a window and an html browser in
> another window to browse the javadocs? ;-)
> 
> Stefano
> 


It's what I'm doing, the confusion comme because I use old javadoc
without correct inherance recopy.

Frankly I think everybody do that when using a simple editor, and I
don't think we need to put all those unnecessary tags.

Julien

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Stefano Bagnara <ap...@bago.org>.
Julien Vermillard ha scritto:
> Well I use vi sometimes and a @see or @inheritedDoc would help. I
> agree with Emmanuel, I don't feel I need to pay something for using
> vi ;)
> 
> And that would make html generated javadoc much readable.

So you would also like to have the list of class implementing a given 
interface in an interface javadoc, and a comment saying that a method 
override a method from the superclass, and a comment saying what are the 
transitive dependencies for the given class, and so on....

Why don't you simply use vi in a window and an html browser in another 
window to browse the javadocs? ;-)

Stefano


Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Julien Vermillard <jv...@archean.fr>.
On Fri, 23 May 2008 12:31:14 +0900
이희승 (Trustin Lee) <tr...@gmail.com> wrote:

> On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny  
> <el...@apache.org> wrote:
> 
> > 이희승 (Trustin Lee) wrote:
> >> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny  
> >> <el...@apache.org> wrote:
> >>
> >>> 이희승 (Trustin Lee) wrote:
> >>>> Every public methods except for the constructors are overridden
> >>>> from its supertypes and interfaces.  They all got proper
> >>>> JavaDoc comments.  Let me know if I am missing something.
> >>>
> >>> Adding a @see Class#method() in the implementation then should
> >>> help. When you look at a method javadoc it's better to know where
> >>> too look at : the intheritance scheme can be feilry complex, and
> >>> it can be a burden to retreive the associated Javadoc.
> >>>
> >>> Something like :
> >>>     /**
> >>>      * @see javax.naming.Context#close()
> >>>      */
> >>>     public void close() throws NamingException
> >>> ...
> >>
> >> I'd just move the cursor on the method?  That shows pretty nicely  
> >> rendered JavaDoc in modern IDEs.
> > Sometime, you just have to use vi or emacs. Make it simple for
> > users : add a @see tag. Cost almost nothing, and it helps.
> 
> I wouldn't bother with vi or emacs.  They pay for what they use.   
> Moreover, it's not 'almost nothing'.
> 
Hi,

Well I use vi sometimes and a @see or @inheritedDoc would help. I
agree with Emmanuel, I don't feel I need to pay something for using
vi ;)

And that would make html generated javadoc much readable.

Julien

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
On Fri, 23 May 2008 12:21:43 +0900, Emmanuel Lecharny  
<el...@apache.org> wrote:

> 이희승 (Trustin Lee) wrote:
>> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny  
>> <el...@apache.org> wrote:
>>
>>> 이희승 (Trustin Lee) wrote:
>>>> Every public methods except for the constructors are overridden from  
>>>> its supertypes and interfaces.  They all got proper JavaDoc  
>>>> comments.  Let me know if I am missing something.
>>>
>>> Adding a @see Class#method() in the implementation then should help.  
>>> When you look at a method javadoc it's better to know where too look  
>>> at : the intheritance scheme can be feilry complex, and it can be a  
>>> burden to retreive the associated Javadoc.
>>>
>>> Something like :
>>>     /**
>>>      * @see javax.naming.Context#close()
>>>      */
>>>     public void close() throws NamingException
>>> ...
>>
>> I'd just move the cursor on the method?  That shows pretty nicely  
>> rendered JavaDoc in modern IDEs.
> Sometime, you just have to use vi or emacs. Make it simple for users :  
> add a @see tag. Cost almost nothing, and it helps.

I wouldn't bother with vi or emacs.  They pay for what they use.   
Moreover, it's not 'almost nothing'.

-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
이희승 (Trustin Lee) wrote:
> On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny 
> <el...@apache.org> wrote:
>
>> 이희승 (Trustin Lee) wrote:
>>> Every public methods except for the constructors are overridden from 
>>> its supertypes and interfaces.  They all got proper JavaDoc 
>>> comments.  Let me know if I am missing something.
>>
>> Adding a @see Class#method() in the implementation then should help. 
>> When you look at a method javadoc it's better to know where too look 
>> at : the intheritance scheme can be feilry complex, and it can be a 
>> burden to retreive the associated Javadoc.
>>
>> Something like :
>>     /**
>>      * @see javax.naming.Context#close()
>>      */
>>     public void close() throws NamingException
>> ...
>
> I'd just move the cursor on the method?  That shows pretty nicely 
> rendered JavaDoc in modern IDEs.
Sometime, you just have to use vi or emacs. Make it simple for users : 
add a @see tag. Cost almost nothing, and it helps.


Thanks.

-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Stefano Bagnara <ap...@bago.org>.
Emmanuel Lecharny ha scritto:
> I withdraw my veto, as the missing @inheritDoc have been added to the file.
> 
> It would have been so much easier, and less noisy to simply do it 
> without arguing, the only discussion would have been about which tag to 
> use (I don't really like the @inheritdoc tag - and I may be wrong about 
> that -, but at least expose to the user that the javadoc is available in 
> an interface or upper class. We may discuss it beside this thread, and 
> inject the result of this discussion into the MINA code best practice).

Sure. If Trustin simply did what you asked with your veto we would have 
a lot of @see or copy&paste comments and it would be a lot worse.
I personally think that @inheritdoc is clutter when you don't need to 
add anything to the doc, but anyway this would have been much easier if 
it didn't start with a veto ;-)

IMHO a similar scenario is better solved with the committers involvement 
and not with a veto from a single PMC member. the community idea about 
the style should be collected and then followed. There could be people 
out there with deep knowledge of javadocs and why a given style is good 
or bad and it is better to discuss new solutions instead of closing in 
your opinions with an early veto.

In my environment the javadoc generation for a class with no javadocs 
implementing an interface with javadocs simply generate this:
-----
Description copied from interface: #InterfaceName#

#InterfaceMethodJavadoc#
------

This also happen for extensions.

The {@inheritdoc} (at least in my environment) simply "hide" the fact 
that the doc is identical to the superclass/interface doc so it reduce 
the available informations.
Furthermore if you use {@inheritdoc} you don't have the link to the 
superclass/interface that you would have with a @see and that is 
automatically added if you don't have a javadoc at all.

That said, is your complaint about the generated javadoc or the code itself?

Stefano


Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
I withdraw my veto, as the missing @inheritDoc have been added to the file.

It would have been so much easier, and less noisy to simply do it 
without arguing, the only discussion would have been about which tag to 
use (I don't really like the @inheritdoc tag - and I may be wrong about 
that -, but at least expose to the user that the javadoc is available in 
an interface or upper class. We may discuss it beside this thread, and 
inject the result of this discussion into the MINA code best practice).

Thanks.

-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
On Fri, 23 May 2008 12:02:45 +0900, Emmanuel Lecharny  
<el...@apache.org> wrote:

> 이희승 (Trustin Lee) wrote:
>> Every public methods except for the constructors are overridden from  
>> its supertypes and interfaces.  They all got proper JavaDoc comments.   
>> Let me know if I am missing something.
>
> Adding a @see Class#method() in the implementation then should help.  
> When you look at a method javadoc it's better to know where too look at  
> : the intheritance scheme can be feilry complex, and it can be a burden  
> to retreive the associated Javadoc.
>
> Something like :
>     /**
>      * @see javax.naming.Context#close()
>      */
>     public void close() throws NamingException
> ...

I'd just move the cursor on the method?  That shows pretty nicely rendered  
JavaDoc in modern IDEs.  If you are interested in the class hierarchy,  
press F4 in Eclipse.  It's what IDEs are supposed to do and they already  
do so.

-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Mike Heath <mh...@apache.org>.
이희승 (Trustin Lee) wrote:
> On Fri, 23 May 2008 14:21:07 +0900, 이희승 (Trustin Lee) 
> <tr...@gmail.com> wrote:
>
>> On Fri, 23 May 2008 13:59:46 +0900, 이희승 (Trustin Lee) 
>> <tr...@gmail.com> wrote:
>>
>>> On Fri, 23 May 2008 13:52:49 +0900, Mike Heath <mh...@apache.org> 
>>> wrote:
>>>
>>>> Emmanuel Lecharny wrote:
>>>>> 이희승 (Trustin Lee) wrote:
>>>>>> Every public methods except for the constructors are overridden 
>>>>>> from its supertypes and interfaces.  They all got proper JavaDoc 
>>>>>> comments.  Let me know if I am missing something.
>>>>>
>>>>> Adding a @see Class#method() in the implementation then should 
>>>>> help. When you look at a method javadoc it's better to know where 
>>>>> too look at : the intheritance scheme can be feilry complex, and 
>>>>> it can be a burden to retreive the associated Javadoc.
>>>>>
>>>>> Something like :
>>>>>    /**
>>>>>     * @see javax.naming.Context#close()
>>>>>     */
>>>>>    public void close() throws NamingException
>>>>> ...
>>>>>
>>>> I think @see is the wrong approach for this situation.  We should 
>>>> use @inheritedDoc instead of @see.  With @see you get no useful 
>>>> information if your IDE supports in-editor popup docs.  You also 
>>>> have to make an extra click to get to useful information when 
>>>> browsing the JavaDocs which is almost as annoying as empty docs.
>>>>
>>>> Also with @inheritedDoc you can see immediately in the code that 
>>>> there are JavaDocs for the method.
>>>>
>>>> IMO using @inheritedDoc instead of empty JavaDocs should be the 
>>>> standard practice in any Java project.
>>>
>>> {@inheritDoc} is a good idea.  However, it's only useful when you 
>>> have something to add to the supertype's documentation.  Otherwise, 
>>> empty JavaDoc is simply replaced with the supertype's documentation 
>>> AFAIK.  Therefore something like the following doesn't have much 
>>> meaning:
>>>
>>>
>>> /**
>>>   * {@inheridDoc}
>>>   */
>>>
>>> Please let me know if I misunderstood how javadoc works.
>>
>> I added {@inheritDoc} tags to some methods and found that 
>> {@inheritDoc} removes 'Description copied from class: SuperType' 
>> message when javadoc is generated.  Other than that, I couldn't find 
>> any difference.  However, I don't like to see that annoying message 
>> with close-to-zero information (i.e. copied from...), so I started to 
>> like to add {@inheritDoc} to all empty JavaDocs.
>>
>> BTW, I found that DefaultByteBufferQueue.size(), iterator() and 
>> peek() have empty documentation in the generated JavaDoc. It's 
>> probably because their documentation is in JDK rather than in local 
>> source code.  Do you know if there is any option that asks javadoc to 
>> fetch JDK documentation?
>
> Google got the answer:
>
> http://forum.java.sun.com/thread.jspa?threadID=5264128&tstart=0
>
Interesting.  Good to know.

-Mike

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
On Fri, 23 May 2008 14:21:07 +0900, 이희승 (Trustin Lee)  
<tr...@gmail.com> wrote:

> On Fri, 23 May 2008 13:59:46 +0900, 이희승 (Trustin Lee)  
> <tr...@gmail.com> wrote:
>
>> On Fri, 23 May 2008 13:52:49 +0900, Mike Heath <mh...@apache.org>  
>> wrote:
>>
>>> Emmanuel Lecharny wrote:
>>>> 이희승 (Trustin Lee) wrote:
>>>>> Every public methods except for the constructors are overridden from  
>>>>> its supertypes and interfaces.  They all got proper JavaDoc  
>>>>> comments.  Let me know if I am missing something.
>>>>
>>>> Adding a @see Class#method() in the implementation then should help.  
>>>> When you look at a method javadoc it's better to know where too look  
>>>> at : the intheritance scheme can be feilry complex, and it can be a  
>>>> burden to retreive the associated Javadoc.
>>>>
>>>> Something like :
>>>>    /**
>>>>     * @see javax.naming.Context#close()
>>>>     */
>>>>    public void close() throws NamingException
>>>> ...
>>>>
>>> I think @see is the wrong approach for this situation.  We should use  
>>> @inheritedDoc instead of @see.  With @see you get no useful  
>>> information if your IDE supports in-editor popup docs.  You also have  
>>> to make an extra click to get to useful information when browsing the  
>>> JavaDocs which is almost as annoying as empty docs.
>>>
>>> Also with @inheritedDoc you can see immediately in the code that there  
>>> are JavaDocs for the method.
>>>
>>> IMO using @inheritedDoc instead of empty JavaDocs should be the  
>>> standard practice in any Java project.
>>
>> {@inheritDoc} is a good idea.  However, it's only useful when you have  
>> something to add to the supertype's documentation.  Otherwise, empty  
>> JavaDoc is simply replaced with the supertype's documentation AFAIK.   
>> Therefore something like the following doesn't have much meaning:
>>
>>
>> /**
>>   * {@inheridDoc}
>>   */
>>
>> Please let me know if I misunderstood how javadoc works.
>
> I added {@inheritDoc} tags to some methods and found that {@inheritDoc}  
> removes 'Description copied from class: SuperType' message when javadoc  
> is generated.  Other than that, I couldn't find any difference.   
> However, I don't like to see that annoying message with close-to-zero  
> information (i.e. copied from...), so I started to like to add  
> {@inheritDoc} to all empty JavaDocs.
>
> BTW, I found that DefaultByteBufferQueue.size(), iterator() and peek()  
> have empty documentation in the generated JavaDoc. It's probably because  
> their documentation is in JDK rather than in local source code.  Do you  
> know if there is any option that asks javadoc to fetch JDK documentation?

Google got the answer:

http://forum.java.sun.com/thread.jspa?threadID=5264128&tstart=0

-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Rich Dougherty <ri...@rd.gen.nz>.
I tend to use @inheritDoc, rather than just omitting javadoc, to show that
I'm intentionally reusing the existing documentation.

I prefer to also avoid manually copying existing documentation, since I
think it's always best to try and avoid duplication where possible. In my
opinion, duplication (of documentation, as well as code) is a big potential
source of errors. Of course, I'm fairly new to collaborative open source
programming, so perhaps there is less chance of error when you have lots of
enthusiastic people reviewing commits. :-)

Cheers
Rich

On Fri, May 23, 2008 at 5:21 PM, 이희승 (Trustin Lee) <tr...@gmail.com>
wrote:

> On Fri, 23 May 2008 13:59:46 +0900, 이희승 (Trustin Lee) <tr...@gmail.com>
> wrote:
>
>  On Fri, 23 May 2008 13:52:49 +0900, Mike Heath <mh...@apache.org> wrote:
>>
>>  Emmanuel Lecharny wrote:
>>>
>>>> 이희승 (Trustin Lee) wrote:
>>>>
>>>>> Every public methods except for the constructors are overridden from
>>>>> its supertypes and interfaces.  They all got proper JavaDoc comments.  Let
>>>>> me know if I am missing something.
>>>>>
>>>>
>>>> Adding a @see Class#method() in the implementation then should help.
>>>> When you look at a method javadoc it's better to know where too look at :
>>>> the intheritance scheme can be feilry complex, and it can be a burden to
>>>> retreive the associated Javadoc.
>>>>
>>>> Something like :
>>>>   /**
>>>>    * @see javax.naming.Context#close()
>>>>    */
>>>>   public void close() throws NamingException
>>>> ...
>>>>
>>>>  I think @see is the wrong approach for this situation.  We should use
>>> @inheritedDoc instead of @see.  With @see you get no useful information if
>>> your IDE supports in-editor popup docs.  You also have to make an extra
>>> click to get to useful information when browsing the JavaDocs which is
>>> almost as annoying as empty docs.
>>>
>>> Also with @inheritedDoc you can see immediately in the code that there
>>> are JavaDocs for the method.
>>>
>>> IMO using @inheritedDoc instead of empty JavaDocs should be the standard
>>> practice in any Java project.
>>>
>>
>> {@inheritDoc} is a good idea.  However, it's only useful when you have
>> something to add to the supertype's documentation.  Otherwise, empty JavaDoc
>> is simply replaced with the supertype's documentation AFAIK.  Therefore
>> something like the following doesn't have much meaning:
>>
>>
>> /**
>>  * {@inheridDoc}
>>  */
>>
>> Please let me know if I misunderstood how javadoc works.
>>
>
> I added {@inheritDoc} tags to some methods and found that {@inheritDoc}
> removes 'Description copied from class: SuperType' message when javadoc is
> generated.  Other than that, I couldn't find any difference.  However, I
> don't like to see that annoying message with close-to-zero information (i.e.
> copied from...), so I started to like to add {@inheritDoc} to all empty
> JavaDocs.
>
> BTW, I found that DefaultByteBufferQueue.size(), iterator() and peek() have
> empty documentation in the generated JavaDoc. It's probably because their
> documentation is in JDK rather than in local source code.  Do you know if
> there is any option that asks javadoc to fetch JDK documentation?
>
>
> Thanks,
> --
> Trustin Lee - Principal Software Engineer, JBoss, Red Hat
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
>

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
On Fri, 23 May 2008 13:59:46 +0900, 이희승 (Trustin Lee)  
<tr...@gmail.com> wrote:

> On Fri, 23 May 2008 13:52:49 +0900, Mike Heath <mh...@apache.org> wrote:
>
>> Emmanuel Lecharny wrote:
>>> 이희승 (Trustin Lee) wrote:
>>>> Every public methods except for the constructors are overridden from  
>>>> its supertypes and interfaces.  They all got proper JavaDoc  
>>>> comments.  Let me know if I am missing something.
>>>
>>> Adding a @see Class#method() in the implementation then should help.  
>>> When you look at a method javadoc it's better to know where too look  
>>> at : the intheritance scheme can be feilry complex, and it can be a  
>>> burden to retreive the associated Javadoc.
>>>
>>> Something like :
>>>    /**
>>>     * @see javax.naming.Context#close()
>>>     */
>>>    public void close() throws NamingException
>>> ...
>>>
>> I think @see is the wrong approach for this situation.  We should use  
>> @inheritedDoc instead of @see.  With @see you get no useful information  
>> if your IDE supports in-editor popup docs.  You also have to make an  
>> extra click to get to useful information when browsing the JavaDocs  
>> which is almost as annoying as empty docs.
>>
>> Also with @inheritedDoc you can see immediately in the code that there  
>> are JavaDocs for the method.
>>
>> IMO using @inheritedDoc instead of empty JavaDocs should be the  
>> standard practice in any Java project.
>
> {@inheritDoc} is a good idea.  However, it's only useful when you have  
> something to add to the supertype's documentation.  Otherwise, empty  
> JavaDoc is simply replaced with the supertype's documentation AFAIK.   
> Therefore something like the following doesn't have much meaning:
>
>
> /**
>   * {@inheridDoc}
>   */
>
> Please let me know if I misunderstood how javadoc works.

I added {@inheritDoc} tags to some methods and found that {@inheritDoc}  
removes 'Description copied from class: SuperType' message when javadoc is  
generated.  Other than that, I couldn't find any difference.  However, I  
don't like to see that annoying message with close-to-zero information  
(i.e. copied from...), so I started to like to add {@inheritDoc} to all  
empty JavaDocs.

BTW, I found that DefaultByteBufferQueue.size(), iterator() and peek()  
have empty documentation in the generated JavaDoc. It's probably because  
their documentation is in JDK rather than in local source code.  Do you  
know if there is any option that asks javadoc to fetch JDK documentation?

Thanks,
-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Niklas Therning <ni...@trillian.se>.
이희승 (Trustin Lee) skrev:
> On Fri, 23 May 2008 13:52:49 +0900, Mike Heath <mh...@apache.org> wrote:
>
>> Emmanuel Lecharny wrote:
>>> 이희승 (Trustin Lee) wrote:
>>>> Every public methods except for the constructors are overridden 
>>>> from its supertypes and interfaces.  They all got proper JavaDoc 
>>>> comments.  Let me know if I am missing something.
>>>
>>> Adding a @see Class#method() in the implementation then should help. 
>>> When you look at a method javadoc it's better to know where too look 
>>> at : the intheritance scheme can be feilry complex, and it can be a 
>>> burden to retreive the associated Javadoc.
>>>
>>> Something like :
>>>    /**
>>>     * @see javax.naming.Context#close()
>>>     */
>>>    public void close() throws NamingException
>>> ...
>>>
>> I think @see is the wrong approach for this situation.  We should use 
>> @inheritedDoc instead of @see.  With @see you get no useful 
>> information if your IDE supports in-editor popup docs.  You also have 
>> to make an extra click to get to useful information when browsing the 
>> JavaDocs which is almost as annoying as empty docs.
>>
>> Also with @inheritedDoc you can see immediately in the code that 
>> there are JavaDocs for the method.
>>
>> IMO using @inheritedDoc instead of empty JavaDocs should be the 
>> standard practice in any Java project.
>
> {@inheritDoc} is a good idea.  However, it's only useful when you have 
> something to add to the supertype's documentation.  Otherwise, empty 
> JavaDoc is simply replaced with the supertype's documentation AFAIK.  
> Therefore something like the following doesn't have much meaning:
>
>
> /**
>  * {@inheridDoc}
>  */
>
> Please let me know if I misunderstood how javadoc works.
>
> Thanks,
>

IMO, omitting the Javadoc comment is better for overriden methods if 
there is nothing additional to say. I don't care about the 'Description 
copied from class: SuperType' message and even just having to type 
@inheritDoc wastes my precious time :-) (ok, we could use a code 
template for that I guess). I agree with Trustin that @inheritDoc is 
useful only when you want to add something to the overridden method's 
comment. Also, I would strongly suggest that we don't use

/**
 * @see ...
 */

as those kinds of comments only mess things up in the final Javadoc 
HTML. They also get in your way if you use a modern IDE (e.g. when 
hovering over a method call I want to see the real javadoc, not just 
"See also...").

/Niklas


Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
On Fri, 23 May 2008 13:52:49 +0900, Mike Heath <mh...@apache.org> wrote:

> Emmanuel Lecharny wrote:
>> 이희승 (Trustin Lee) wrote:
>>> Every public methods except for the constructors are overridden from  
>>> its supertypes and interfaces.  They all got proper JavaDoc comments.   
>>> Let me know if I am missing something.
>>
>> Adding a @see Class#method() in the implementation then should help.  
>> When you look at a method javadoc it's better to know where too look at  
>> : the intheritance scheme can be feilry complex, and it can be a burden  
>> to retreive the associated Javadoc.
>>
>> Something like :
>>    /**
>>     * @see javax.naming.Context#close()
>>     */
>>    public void close() throws NamingException
>> ...
>>
> I think @see is the wrong approach for this situation.  We should use  
> @inheritedDoc instead of @see.  With @see you get no useful information  
> if your IDE supports in-editor popup docs.  You also have to make an  
> extra click to get to useful information when browsing the JavaDocs  
> which is almost as annoying as empty docs.
>
> Also with @inheritedDoc you can see immediately in the code that there  
> are JavaDocs for the method.
>
> IMO using @inheritedDoc instead of empty JavaDocs should be the standard  
> practice in any Java project.

{@inheritDoc} is a good idea.  However, it's only useful when you have  
something to add to the supertype's documentation.  Otherwise, empty  
JavaDoc is simply replaced with the supertype's documentation AFAIK.   
Therefore something like the following doesn't have much meaning:


/**
  * {@inheridDoc}
  */

Please let me know if I misunderstood how javadoc works.

Thanks,

-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Mike Heath <mh...@apache.org>.
Emmanuel Lecharny wrote:
> 이희승 (Trustin Lee) wrote:
>> Every public methods except for the constructors are overridden from 
>> its supertypes and interfaces.  They all got proper JavaDoc 
>> comments.  Let me know if I am missing something.
>
> Adding a @see Class#method() in the implementation then should help. 
> When you look at a method javadoc it's better to know where too look 
> at : the intheritance scheme can be feilry complex, and it can be a 
> burden to retreive the associated Javadoc.
>
> Something like :
>    /**
>     * @see javax.naming.Context#close()
>     */
>    public void close() throws NamingException
> ...
>
I think @see is the wrong approach for this situation.  We should use 
@inheritedDoc instead of @see.  With @see you get no useful information 
if your IDE supports in-editor popup docs.  You also have to make an 
extra click to get to useful information when browsing the JavaDocs 
which is almost as annoying as empty docs.

Also with @inheritedDoc you can see immediately in the code that there 
are JavaDocs for the method.

IMO using @inheritedDoc instead of empty JavaDocs should be the standard 
practice in any Java project.

-Mike

Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by Emmanuel Lecharny <el...@apache.org>.
이희승 (Trustin Lee) wrote:
> Every public methods except for the constructors are overridden from 
> its supertypes and interfaces.  They all got proper JavaDoc comments.  
> Let me know if I am missing something.

Adding a @see Class#method() in the implementation then should help. 
When you look at a method javadoc it's better to know where too look at 
: the intheritance scheme can be feilry complex, and it can be a burden 
to retreive the associated Javadoc.

Something like :
    /**
     * @see javax.naming.Context#close()
     */
    public void close() throws NamingException
...

-- 
--
cordialement, regards,
Emmanuel Lécharny
www.iktek.com
directory.apache.org



Re: svn commit: r659372 - /mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java

Posted by "이희승 (Trustin Lee)" <tr...@gmail.com>.
Every public methods except for the constructors are overridden from its  
supertypes and interfaces.  They all got proper JavaDoc comments.  Let me  
know if I am missing something.

On Fri, 23 May 2008 11:41:05 +0900, Emmanuel Lecharny  
<el...@apache.org> wrote:

> Trustin,
>
> can you add at least a simple Javadoc for every public method before  
> committing ?
>
> Committing code omitting javadoc is just a bad habit : you will have to  
> add the Javadoc anyway later, but it will cost you more effort and more  
> pain than doing it right away.
>
>
> Thanks !
>
> trustin@apache.org wrote:
>> Author: trustin
>> Date: Thu May 22 19:35:19 2008
>> New Revision: 659372
>>
>> URL: http://svn.apache.org/viewvc?rev=659372&view=rev
>> Log:
>> Added the default implementation of ByteBufferQueue - need to write a  
>> test case
>>
>> Added:
>>     mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java    
>> (with props)
>>
>> Added:  
>> mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java
>> URL:  
>> http://svn.apache.org/viewvc/mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java?rev=659372&view=auto
>> ==============================================================================
>> ---  
>> mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java  
>> (added)
>> +++  
>> mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java  
>> Thu May 22 19:35:19 2008
>> @@ -0,0 +1,854 @@
>> +/*
>> + *  Licensed to the Apache Software Foundation (ASF) under one
>> + *  or more contributor license agreements.  See the NOTICE file
>> + *  distributed with this work for additional information
>> + *  regarding copyright ownership.  The ASF licenses this file
>> + *  to you under the Apache License, Version 2.0 (the
>> + *  "License"); you may not use this file except in compliance
>> + *  with the License.  You may obtain a copy of the License at
>> + *
>> + *    http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + *  Unless required by applicable law or agreed to in writing,
>> + *  software distributed under the License is distributed on an
>> + *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
>> + *  KIND, either express or implied.  See the License for the
>> + *  specific language governing permissions and limitations
>> + *  under the License.
>> + *
>> + */
>> +package org.apache.mina.queue;
>> +
>> +import java.nio.ByteBuffer;
>> +import java.nio.ByteOrder;
>> +import java.util.ConcurrentModificationException;
>> +import java.util.Iterator;
>> +import java.util.NoSuchElementException;
>> +import java.util.Queue;
>> +
>> +/**
>> + * The default {@link ByteBufferQueue} implementation.
>> + *
>> + * @author The Apache MINA project (dev@mina.apache.org)
>> + * @version $Rev$, $Date$
>> + */
>> +public class DefaultByteBufferQueue extends  
>> AbstractIoQueue<ByteBuffer> implements
>> +        ByteBufferQueue {
>> +
>> +    private static final int DEFAULT_CAPACITY_INCREMENT = 512;
>> +
>> +    private final Queue<ByteBuffer> queue;
>> +    private final ByteOrder order;
>> +    private final ByteBufferFactory bufferFactory;
>> +    private final int capacityIncrement;
>> +    private ByteBuffer tail;
>> +    private int length;
>> +
>> +    public DefaultByteBufferQueue() {
>> +        this(
>> +                new CircularQueue<ByteBuffer>(), ByteOrder.BIG_ENDIAN,
>> +                HeapByteBufferFactory.INSTANCE,  
>> DEFAULT_CAPACITY_INCREMENT);
>> +    }
>> +
>> +    public DefaultByteBufferQueue(int capacityIncrement) {
>> +        this(
>> +                new CircularQueue<ByteBuffer>(), ByteOrder.BIG_ENDIAN,
>> +                HeapByteBufferFactory.INSTANCE, capacityIncrement);
>> +    }
>> +
>> +    public DefaultByteBufferQueue(ByteOrder order) {
>> +        this(
>> +                new CircularQueue<ByteBuffer>(), order,
>> +                HeapByteBufferFactory.INSTANCE,  
>> DEFAULT_CAPACITY_INCREMENT);
>> +    }
>> +
>> +    public DefaultByteBufferQueue(ByteOrder order, int  
>> capacityIncrement) {
>> +        this(
>> +                new CircularQueue<ByteBuffer>(), order,
>> +                HeapByteBufferFactory.INSTANCE, capacityIncrement);
>> +    }
>> +
>> +    public DefaultByteBufferQueue(
>> +            Queue<ByteBuffer> queue, ByteOrder order,
>> +            ByteBufferFactory bufferFactory, int capacityIncrement) {
>> +
>> +        if (queue == null) {
>> +            throw new NullPointerException("queue");
>> +        }
>> +        if (order == null) {
>> +            throw new NullPointerException("order");
>> +        }
>> +        if (bufferFactory == null) {
>> +            throw new NullPointerException("bufferFactory");
>> +        }
>> +        if (capacityIncrement < 8) {
>> +            throw new IllegalArgumentException(
>> +                    "capacityIncrement: " + capacityIncrement +
>> +                    " (expected: an integer greater than or equals to  
>> 8)");
>> +        }
>> +
>> +        this.queue = queue;
>> +        this.order = order;
>> +        this.bufferFactory = bufferFactory;
>> +        this.capacityIncrement = capacityIncrement;
>> +    }
>> +
>> +    @Override
>> +    protected boolean doOffer(ByteBuffer e) {
>> +        e = e.duplicate();
>> +
>> +        // Refuse to add an empty buffer.
>> +        if (!e.hasRemaining()) {
>> +            return false;
>> +        }
>> +
>> +        tail = null;
>> +        e.order(order);
>> +        return queue.offer(e);
>> +    }
>> +
>> +    @Override
>> +    protected ByteBuffer doPoll() {
>> +        ByteBuffer buf = queue.poll();
>> +        if (buf == tail) {
>> +            tail = null;
>> +        }
>> +        return buf;
>> +    }
>> +
>> +    @Override
>> +    public Iterator<ByteBuffer> iterator() {
>> +        final Iterator<ByteBuffer> i = queue.iterator();
>> +        return new Iterator<ByteBuffer>() {
>> +            public boolean hasNext() {
>> +                return i.hasNext();
>> +            }
>> +
>> +            public ByteBuffer next() {
>> +                return i.next();
>> +            }
>> +
>> +            public void remove() {
>> +                throw new UnsupportedOperationException();
>> +            }
>> +        };
>> +    }
>> +
>> +    @Override
>> +    public int size() {
>> +        return queue.size();
>> +    }
>> +
>> +    public ByteBuffer peek() {
>> +        return queue.peek();
>> +    }
>> +
>> +    public ByteOrder order() {
>> +        return order;
>> +    }
>> +
>> +    public int length() {
>> +        return length;
>> +    }
>> +
>> +    public void addByte(byte value) {
>> +        if (!offerByte(value)) {
>> +            throw new IllegalStateException();
>> +        }
>> +    }
>> +
>> +    public void addShort(short value) {
>> +        if (!offerShort(value)) {
>> +            throw new IllegalStateException();
>> +        }
>> +    }
>> +
>> +    public void addInt(int value) {
>> +        if (!offerInt(value)) {
>> +            throw new IllegalStateException();
>> +        }
>> +    }
>> +
>> +    public void addLong(long value) {
>> +        if (!offerLong(value)) {
>> +            throw new IllegalStateException();
>> +        }
>> +    }
>> +
>> +    public void addFloat(float value) {
>> +        if (!offerFloat(value)) {
>> +            throw new IllegalStateException();
>> +        }
>> +    }
>> +
>> +    public void addDouble(double value) {
>> +        if (!offerDouble(value)) {
>> +            throw new IllegalStateException();
>> +        }
>> +    }
>> +
>> +    public boolean offerByte(byte value) {
>> +        ByteBuffer tail = tail(1);
>> +        if (tail == null) {
>> +            return false;
>> +        }
>> +        int index = tail.limit();
>> +        tail.limit(index + 1);
>> +        tail.put(index, value);
>> +        length ++;
>> +        return true;
>> +    }
>> +
>> +    public boolean offerShort(short value) {
>> +        ByteBuffer tail = tail(2);
>> +        if (tail == null) {
>> +            return false;
>> +        }
>> +        int index = tail.limit();
>> +        tail.limit(index + 2);
>> +        tail.putShort(index, value);
>> +        length += 2;
>> +        return true;
>> +    }
>> +
>> +    public boolean offerInt(int value) {
>> +        ByteBuffer tail = tail(4);
>> +        if (tail == null) {
>> +            return false;
>> +        }
>> +        int index = tail.limit();
>> +        tail.limit(index + 4);
>> +        tail.putInt(index, value);
>> +        length += 4;
>> +        return true;
>> +    }
>> +
>> +    public boolean offerLong(long value) {
>> +        ByteBuffer tail = tail(8);
>> +        if (tail == null) {
>> +            return false;
>> +        }
>> +        int index = tail.limit();
>> +        tail.limit(index + 8);
>> +        tail.putLong(index, value);
>> +        length += 8;
>> +        return true;
>> +    }
>> +
>> +    public boolean offerFloat(float value) {
>> +        return offerInt(Float.floatToIntBits(value));
>> +    }
>> +
>> +    public boolean offerDouble(double value) {
>> +        return offerLong(Double.doubleToLongBits(value));
>> +    }
>> +
>> +    public boolean pollSlice(Queue<ByteBuffer> destination, int  
>> length) {
>> +        if (length < 0) {
>> +            throw new IllegalArgumentException(
>> +                    "length: " + length +
>> +                    " (expected: zero or a positive integer)");
>> +        }
>> +
>> +        if (length > this.length) {
>> +            return false;
>> +        }
>> +
>> +        if (length == 0) {
>> +            return true;
>> +        }
>> +
>> +        int bytesToRead = length;
>> +        for (;;) {
>> +            ByteBuffer element = queue.peek();
>> +            if (element == null) {
>> +                // element shouldn't be null unless it's accessed  
>> concurrently.
>> +                throw new ConcurrentModificationException();
>> +            }
>> +
>> +            int remaining = element.remaining();
>> +            if (remaining == 0) {
>> +                removeAndAssert(element);
>> +                continue;
>> +            }
>> +
>> +            if (remaining >= bytesToRead) {
>> +                int position = element.position();
>> +                ByteBuffer lastElement = element.duplicate();
>> +                lastElement.limit(position + bytesToRead);
>> +                if (destination.offer(lastElement)) {
>> +                    element.position(position + bytesToRead);
>> +                    this.length -= bytesToRead;
>> +                    return true;
>> +                } else {
>> +                    return false;
>> +                }
>> +            }
>> +
>> +            if (destination.offer(element.duplicate())) {
>> +                removeAndAssert(element);
>> +                element.limit(element.limit());
>> +                this.length -= remaining;
>> +                bytesToRead -= remaining;
>> +            } else {
>> +                return false;
>> +            }
>> +        }
>> +    }
>> +
>> +    public boolean peekSlice(Queue<ByteBuffer> destination, int  
>> length) {
>> +        if (length < 0) {
>> +            throw new IllegalArgumentException(
>> +                    "length: " + length +
>> +                    " (expected: zero or a positive integer)");
>> +        }
>> +
>> +        if (length > this.length) {
>> +            return false;
>> +        }
>> +
>> +        // No need to fetch anything if the specified length is zero.
>> +        if (length == 0) {
>> +            return true;
>> +        }
>> +
>> +        // Optimize when it's enough with one slice.
>> +        ByteBuffer element = safeElement();
>> +        if (element.remaining() >= length) {
>> +            ByteBuffer lastElement = element.duplicate();
>> +            lastElement.limit(element.position() + length);
>> +            return destination.offer(lastElement);
>> +        }
>> +
>> +        // Otherwise we have to use an iterator.
>> +        int bytesToRead = length;
>> +        for (ByteBuffer e: this) {
>> +            int remaining = e.remaining();
>> +            if (remaining == 0) {
>> +                continue;
>> +            }
>> +
>> +            if (remaining >= bytesToRead) {
>> +                ByteBuffer lastElement = element.duplicate();
>> +                lastElement.limit(e.position() + bytesToRead);
>> +                if (!destination.offer(lastElement)) {
>> +                    return false;
>> +                } else {
>> +                    return true;
>> +                }
>> +            }
>> +
>> +            if (!destination.offer(element)) {
>> +                return false;
>> +            }
>> +            bytesToRead -= remaining;
>> +        }
>> +
>> +        // The only case that we reach here is concurrent access.
>> +        throw new ConcurrentModificationException();
>> +    }
>> +
>> +    public boolean elementAsSlice(Queue<ByteBuffer> destination, int  
>> length) {
>> +        checkSequentialAccess(length);
>> +        return peekSlice(destination, length);
>> +    }
>> +
>> +    public boolean removeSlice(Queue<ByteBuffer> destination, int  
>> length) {
>> +        checkSequentialAccess(length);
>> +        return pollSlice(destination, length);
>> +    }
>> +
>> +    public boolean getSlice(Queue<ByteBuffer> destination, int  
>> byteIndex, int length) {
>> +        checkRandomAccess(byteIndex, length);
>> +
>> +        // No need to fetch anything if the specified length is zero.
>> +        if (length == 0) {
>> +            return true;
>> +        }
>> +
>> +        if (byteIndex == 0) {
>> +            return elementAsSlice(destination, length);
>> +        }
>> +
>> +        // Optimize when it's enough with one slice.
>> +        ByteBuffer element = safeElement();
>> +        if (element.remaining() >= byteIndex + length) {
>> +            ByteBuffer lastElement = element.duplicate();
>> +            lastElement.position(lastElement.position() + byteIndex);
>> +            lastElement.limit(lastElement.position() + length);
>> +            return destination.offer(lastElement);
>> +        }
>> +
>> +        // Otherwise we have to use an iterator.
>> +        int bytesToRead = length;
>> +        for (ByteBuffer b: this) {
>> +            int remaining = b.remaining();
>> +
>> +            // Skip until we find the element which matches to the  
>> offset.
>> +            if (remaining <= byteIndex) {
>> +                byteIndex -= remaining;
>> +                continue;
>> +            }
>> +
>> +            if (remaining > byteIndex + length) {
>> +                ByteBuffer lastElement = b.duplicate();
>> +                lastElement.position(lastElement.position() +  
>> byteIndex);
>> +                lastElement.limit(lastElement.position() + length);
>> +                return destination.offer(lastElement);
>> +            }
>> +
>> +            b = b.duplicate();
>> +            b.position(b.position() + byteIndex);
>> +            destination.offer(b);
>> +            bytesToRead -= remaining - byteIndex;
>> +            byteIndex -= remaining - byteIndex;
>> +        }
>> +
>> +        throw new ConcurrentModificationException();
>> +    }
>> +
>> +    public byte   removeByte() {
>> +        checkSequentialAccess(1);
>> +        ByteBuffer e = safeElement();
>> +        length --;
>> +        byte value = e.get();
>> +
>> +        if (!e.hasRemaining()) {
>> +            trim();
>> +        }
>> +        return value;
>> +    }
>> +
>> +    public short  removeShort() {
>> +        checkSequentialAccess(2);
>> +        // Try to read in one shot.
>> +        ByteBuffer e = safeElement();
>> +        int remaining = e.remaining();
>> +        switch (remaining) {
>> +        case 0: case 1:
>> +            break;
>> +        case 2:
>> +            length -= 2;
>> +            short value = e.getShort();
>> +            trim();
>> +            return value;
>> +        default:
>> +            length -= 2;
>> +            return e.getShort();
>> +        }
>> +
>> +        // Otherwise, read byte by byte. (inefficient!)
>> +        int value = 0;
>> +        for (int bytesToRead = 2; bytesToRead > 0; bytesToRead --) {
>> +            value = value << 8 | e.get();
>> +            remaining --;
>> +            length --;
>> +            if (remaining == 0) {
>> +                e = safeElement();
>> +                remaining = e.remaining();
>> +            }
>> +        }
>> +
>> +        return applyByteOrder((short) value);
>> +    }
>> +
>> +    public int    removeInt() {
>> +        checkSequentialAccess(4);
>> +        // Try to read in one shot.
>> +        ByteBuffer e = safeElement();
>> +        int remaining = e.remaining();
>> +        switch (remaining) {
>> +        case 0: case 1: case 2: case 3:
>> +            break;
>> +        case 4:
>> +            length -= 4;
>> +            int value = e.getInt();
>> +            trim();
>> +            return value;
>> +        default:
>> +            length -= 4;
>> +            return e.getInt();
>> +        }
>> +
>> +        // Otherwise, read byte by byte. (inefficient!)
>> +        int value = 0;
>> +        for (int bytesToRead = 4; bytesToRead > 0; bytesToRead --) {
>> +            value = value << 8 | e.get();
>> +            length --;
>> +            remaining --;
>> +            if (remaining == 0) {
>> +                e = safeElement();
>> +                remaining = e.remaining();
>> +            }
>> +        }
>> +
>> +        return applyByteOrder(value);
>> +    }
>> +
>> +    public long   removeLong() {
>> +        checkSequentialAccess(8);
>> +        // Try to read in one shot.
>> +        ByteBuffer e = safeElement();
>> +        int remaining = e.remaining();
>> +        switch (remaining) {
>> +        case 0: case 1: case 2: case 3: case 4: case 5: case 6: case 7:
>> +            break;
>> +        case 8:
>> +            length -= 8;
>> +            long value = e.getLong();
>> +            trim();
>> +            return value;
>> +        default:
>> +            length -= 8;
>> +            return e.getLong();
>> +        }
>> +
>> +        // Otherwise, read byte by byte. (inefficient!)
>> +        long value = 0;
>> +        for (int bytesToRead = 8; bytesToRead > 0; bytesToRead --) {
>> +            value = value << 8 | e.get();
>> +            length --;
>> +            remaining --;
>> +            if (remaining == 0) {
>> +                e = safeElement();
>> +                remaining = e.remaining();
>> +            }
>> +        }
>> +
>> +        return applyByteOrder(value);
>> +    }
>> +
>> +    public float  removeFloat() {
>> +        return Float.intBitsToFloat(removeInt());
>> +    }
>> +
>> +    public double removeDouble() {
>> +        return Double.longBitsToDouble(removeLong());
>> +    }
>> +
>> +    public void   discard(int length) {
>> +        checkSequentialAccess(length);
>> +
>> +        int bytesToDiscard = length;
>> +        while (bytesToDiscard > 0) {
>> +            ByteBuffer element = queue.peek();
>> +            int remaining = element.remaining();
>> +            if (remaining == 0) {
>> +                removeAndAssert(element);
>> +                continue;
>> +            }
>> +
>> +            if (remaining >= bytesToDiscard) {
>> +                element.position(element.position() + bytesToDiscard);
>> +                this.length -= bytesToDiscard;
>> +                break;
>> +            }
>> +
>> +            removeAndAssert(element);
>> +            element.limit(element.limit());
>> +            bytesToDiscard -= remaining;
>> +            this.length -= remaining;
>> +        }
>> +    }
>> +
>> +    public byte   elementAsByte  () {
>> +        checkSequentialAccess(1);
>> +        ByteBuffer e = safeElement();
>> +        return e.get(e.position());
>> +    }
>> +
>> +    public short  elementAsShort () {
>> +        checkSequentialAccess(2);
>> +        // Try to read in one shot.
>> +        ByteBuffer e = safeElement();
>> +        if (e.remaining() >= 2) {
>> +            return e.getShort(e.position());
>> +        }
>> +
>> +        // Otherwise, read byte by byte. (inefficient!)
>> +        return applyByteOrder((short) readByteByByte(2));
>> +    }
>> +
>> +    public int    elementAsInt   () {
>> +        checkSequentialAccess(4);
>> +        // Try to read in one shot.
>> +        ByteBuffer e = safeElement();
>> +        if (e.remaining() >= 4) {
>> +            return e.getInt(e.position());
>> +        }
>> +
>> +        // Otherwise, read byte by byte. (inefficient!)
>> +        return applyByteOrder((int) readByteByByte(4));
>> +    }
>> +
>> +    public long   elementAsLong  () {
>> +        checkSequentialAccess(8);
>> +        // Try to read in one shot.
>> +        ByteBuffer e = safeElement();
>> +        if (e.remaining() >= 8) {
>> +            return e.getLong(e.position());
>> +        }
>> +
>> +        // Otherwise, read byte by byte. (inefficient!)
>> +        return applyByteOrder(readByteByByte(8));
>> +    }
>> +
>> +    public float  elementAsFloat () {
>> +        return Float.intBitsToFloat(elementAsInt());
>> +    }
>> +
>> +    public double elementAsDouble() {
>> +        return Double.longBitsToDouble(elementAsLong());
>> +    }
>> +
>> +    public byte   getByte  (int byteIndex) {
>> +        checkRandomAccess(byteIndex, 1);
>> +        if (byteIndex == 0) {
>> +            return elementAsByte();
>> +        }
>> +
>> +        // Get the value from the first element if possible.
>> +        ByteBuffer e = safeElement();
>> +        if (e.remaining() > byteIndex) {
>> +            return e.get(e.position() + byteIndex);
>> +        }
>> +
>> +        // Otherwise, start expensive traversal.
>> +        for (ByteBuffer b: this) {
>> +            if (b.remaining() > byteIndex) {
>> +                return e.get(e.position() + byteIndex);
>> +            } else {
>> +                byteIndex -= e.remaining();
>> +            }
>> +        }
>> +
>> +        // Should never reach here unless concurrent modification  
>> happened.
>> +        throw new ConcurrentModificationException();
>> +    }
>> +
>> +    public short  getShort (int byteIndex) {
>> +        checkRandomAccess(byteIndex, 2);
>> +        if (byteIndex == 0) {
>> +            return elementAsByte();
>> +        }
>> +
>> +        // Get the value from the first element if possible.
>> +        ByteBuffer e = safeElement();
>> +        if (e.remaining() >= byteIndex + 2) {
>> +            return e.getShort(e.position() + byteIndex);
>> +        }
>> +
>> +        // Otherwise, start expensive traversal.
>> +        return applyByteOrder((short) getByteByByte(byteIndex, 2));
>> +    }
>> +
>> +    public int    getInt   (int byteIndex) {
>> +        checkRandomAccess(byteIndex, 4);
>> +        if (byteIndex == 0) {
>> +            return elementAsByte();
>> +        }
>> +
>> +        // Get the value from the first element if possible.
>> +        ByteBuffer e = safeElement();
>> +        if (e.remaining() >= byteIndex + 4) {
>> +            return e.getInt(e.position() + byteIndex);
>> +        }
>> +
>> +        // Otherwise, start expensive traversal.
>> +        return applyByteOrder((int) getByteByByte(byteIndex, 4));
>> +    }
>> +
>> +    public long   getLong  (int byteIndex) {
>> +        checkRandomAccess(byteIndex, 8);
>> +        if (byteIndex == 0) {
>> +            return elementAsByte();
>> +        }
>> +
>> +        // Get the value from the first element if possible.
>> +        ByteBuffer e = safeElement();
>> +        if (e.remaining() >= byteIndex + 8) {
>> +            return e.getLong(e.position() + byteIndex);
>> +        }
>> +
>> +        // Otherwise, start expensive traversal.
>> +        return applyByteOrder(getByteByByte(byteIndex, 8));
>> +    }
>> +
>> +    public float  getFloat (int byteIndex) {
>> +        return Float.intBitsToFloat(getInt(byteIndex));
>> +    }
>> +
>> +    public double getDouble(int byteIndex) {
>> +        return Double.longBitsToDouble(getLong(byteIndex));
>> +    }
>> +
>> +    public ByteBuffer merge() {
>> +        ByteBuffer buf = bufferFactory.newByteBuffer(length);
>> +        buf.order(order);
>> +        for (ByteBuffer e: queue) {
>> +            buf.put(e.duplicate());
>> +        }
>> +        buf.clear();
>> +        return buf;
>> +    }
>> +
>> +    private ByteBuffer tail(int length) {
>> +        ByteBuffer oldTail = tail;
>> +        if (oldTail == null || oldTail.capacity() - oldTail.limit() <  
>> length) {
>> +            ByteBuffer newTail =  
>> bufferFactory.newByteBuffer(capacityIncrement);
>> +            newTail.order(order);
>> +            newTail.limit(0);
>> +            if (!queue.offer(newTail)) {
>> +                return null;
>> +            }
>> +            tail = newTail;
>> +            return newTail;
>> +        } else {
>> +            return oldTail;
>> +        }
>> +    }
>> +
>> +    /**
>> +     * The same operation with {@link #element()} except that it never  
>> returns
>> +     * an empty buffer.
>> +     */
>> +    private ByteBuffer safeElement() {
>> +        for (;;) {
>> +            ByteBuffer e = queue.element();
>> +            if (e.hasRemaining()) {
>> +                return e;
>> +            } else {
>> +                removeAndAssert(e);
>> +            }
>> +        }
>> +    }
>> +
>> +    /**
>> +     * Removes any empty buffers in the head of this queue.
>> +     */
>> +    private void trim() {
>> +        for (;;) {
>> +            ByteBuffer e = queue.peek();
>> +            if (e == null || e.hasRemaining()) {
>> +                break;
>> +            }
>> +            removeAndAssert(e);
>> +        }
>> +    }
>> +
>> +    /**
>> +     * Removes the first element and make sure it is same with the  
>> specified
>> +     * buffer.
>> +     */
>> +    private void removeAndAssert(ByteBuffer e) {
>> +        ByteBuffer removedElement = queue.remove();
>> +        assert removedElement == e;
>> +    }
>> +
>> +    /**
>> +     * Throws an exception if the specified length is illegal or the  
>> length of
>> +     * this queue is less than the specified integer.
>> +     */
>> +    private void checkSequentialAccess(int length) {
>> +        if (length < 0) {
>> +            throw new IllegalArgumentException(
>> +                    "length: " + length +
>> +                    " (expected: zero or a positive integer)");
>> +        }
>> +
>> +        if (this.length < length) {
>> +            throw new NoSuchElementException();
>> +        }
>> +    }
>> +
>> +    /**
>> +     * Throws an exception if the byteIndex is incorrect or the length  
>> of this
>> +     * queue is less than the specified integer + byteIndex.
>> +     */
>> +    private void checkRandomAccess(int byteIndex, int length) {
>> +        if (byteIndex < 0) {
>> +            throw new IllegalArgumentException(
>> +                    "byteIndex: " + byteIndex +
>> +                    " (expected: 0 or a positive integer)");
>> +        }
>> +
>> +        if (this.length < byteIndex + length) {
>> +            throw new IndexOutOfBoundsException();
>> +        }
>> +    }
>> +
>> +    private long readByteByByte(int bytesToRead) {
>> +        long value = 0;
>> +        for (ByteBuffer b: this) {
>> +            int remaining = b.remaining();
>> +            for (int i = 0; i < remaining; i ++) {
>> +                value = value << 8 | b.get(b.position() + i);
>> +                bytesToRead --;
>> +                if (bytesToRead == 0) {
>> +                    return value;
>> +                }
>> +            }
>> +        }
>> +
>> +        throw new ConcurrentModificationException();
>> +    }
>> +
>> +    private long getByteByByte(int byteIndex, int bytesToRead) {
>> +        long value = 0;
>> +        for (ByteBuffer b: this) {
>> +            int remaining = b.remaining();
>> +
>> +            // Skip until we find the element which matches to the  
>> offset.
>> +            if (remaining <= byteIndex) {
>> +                byteIndex -= remaining;
>> +                continue;
>> +            }
>> +
>> +            for (int i = 0; i < remaining; i ++) {
>> +                value = value << 8 | b.get(b.position() + byteIndex);
>> +                bytesToRead --;
>> +                if (bytesToRead == 0) {
>> +                    return value;
>> +                }
>> +                byteIndex ++;
>> +            }
>> +            byteIndex -= remaining;
>> +        }
>> +        throw new ConcurrentModificationException();
>> +    }
>> +
>> +    private short applyByteOrder(short value) {
>> +        // Reverse the bytes if necessary.
>> +        if (order == ByteOrder.LITTLE_ENDIAN) {
>> +            int newValue = value >>> 8 & 0xFF | (value & 0xFF) << 8;
>> +            value = (short) newValue;
>> +        }
>> +        return value;
>> +    }
>> +
>> +    private int applyByteOrder(int value) {
>> +        // Reverse the bytes if necessary.
>> +        if (order == ByteOrder.LITTLE_ENDIAN) {
>> +            value = (value >>> 24 & 0xFF) <<  0 |
>> +                    (value >>> 16 & 0xFF) <<  8 |
>> +                    (value >>>  8 & 0xFF) << 16 |
>> +                    (value >>>  0 & 0xFF) << 24;
>> +        }
>> +        return value;
>> +    }
>> +
>> +    private long applyByteOrder(long value) {
>> +        // Reverse the bytes if necessary.
>> +        if (order == ByteOrder.LITTLE_ENDIAN) {
>> +            value = (value >>> 56 & 0xFFL) <<  0 |
>> +                    (value >>> 48 & 0xFFL) <<  8 |
>> +                    (value >>> 40 & 0xFFL) << 16 |
>> +                    (value >>> 32 & 0xFFL) << 24 |
>> +                    (value >>> 24 & 0xFFL) << 32 |
>> +                    (value >>> 16 & 0xFFL) << 40 |
>> +                    (value >>>  8 & 0xFFL) << 48 |
>> +                    (value >>>  0 & 0xFFL) << 56;
>> +        }
>> +        return value;
>> +    }
>> +}
>>
>> Propchange:  
>> mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java
>> ------------------------------------------------------------------------------
>>     svn:eol-style = native
>>
>> Propchange:  
>> mina/branches/buffer/core/src/main/java/org/apache/mina/queue/DefaultByteBufferQueue.java
>> ------------------------------------------------------------------------------
>>     svn:keywords = Rev Date
>>
>>
>>
>>
>
>



-- 
Trustin Lee - Principal Software Engineer, JBoss, Red Hat
--
what we call human nature is actually human habit
--
http://gleamynode.net/