メンバーの順序

意見に対して別の論を示したら強烈にdisられてしまいました。議論にならないのは残念ですね。

ともあれ、いい機会なのでメンバーの順序について掘り下げて考えたいと思います。といっても明確な結論は出なかったので、事実の確認と思考のメモということで。

話の発端になったコメントで、私は定義→使用の順で書いた方がいい、と書きました。

Enclosingクラスのデータ構造に密接に関わるようなInnerクラスの場合、最初に明示された方が全体のデータ構造が見通しやすいように思えます。
これは concurrent library でも有名な Doug Lea氏などもこう書いていますね。

JavaSE6.0 java.util.concurrent.atomic.AtomicStampedReference
public class AtomicStampedReference<V>  {

    private static class ReferenceIntegerPair<T> {
        private final T reference;
        private final int integer;
        ReferenceIntegerPair(T r, int i) {
            reference = r; integer = i;
        }
    }

    private final AtomicReference<ReferenceIntegerPair<V>>  atomicRef;

    public AtomicStampedReference(V initialRef, int initialStamp) {
        atomicRef = new AtomicReference<ReferenceIntegerPair<V>>
            (new ReferenceIntegerPair<V>(initialRef, initialStamp));
    }

    public V getReference() {
        return atomicRef.get().reference;
    }

    public int getStamp() {
        return atomicRef.get().integer;
    }

    // 以下メソッド省略

引用にあたりコメントは省略しました。

もう一個例を。

JavaSE6.0 java.util.concurrent.ConcurrentLinkedQueue
public class ConcurrentLinkedQueue<E> extends AbstractQueue<E>
        implements Queue<E>, java.io.Serializable {
    private static final long serialVersionUID = 196745693267521676L;

    private static class Node<E> {
        private volatile E item;
        private volatile Node<E> next;

        private static final
            AtomicReferenceFieldUpdater<Node, Node>
            nextUpdater =
            AtomicReferenceFieldUpdater.newUpdater
            (Node.class, Node.class, "next");
        private static final
            AtomicReferenceFieldUpdater<Node, Object>
            itemUpdater =
            AtomicReferenceFieldUpdater.newUpdater
            (Node.class, Object.class, "item");

        Node(E x) { item = x; }

        Node(E x, Node<E> n) { item = x; next = n; }

        E getItem() {
            return item;
        }

        boolean casItem(E cmp, E val) {
            return itemUpdater.compareAndSet(this, cmp, val);
        }

        void setItem(E val) {
            itemUpdater.set(this, val);
        }

        Node<E> getNext() {
            return next;
        }

        boolean casNext(Node<E> cmp, Node<E> val) {
            return nextUpdater.compareAndSet(this, cmp, val);
        }

        void setNext(Node<E> val) {
            nextUpdater.set(this, val);
        }

    }

    private static final
        AtomicReferenceFieldUpdater<ConcurrentLinkedQueue, Node>
        tailUpdater =
        AtomicReferenceFieldUpdater.newUpdater
        (ConcurrentLinkedQueue.class, Node.class, "tail");
    private static final
        AtomicReferenceFieldUpdater<ConcurrentLinkedQueue, Node>
        headUpdater =
        AtomicReferenceFieldUpdater.newUpdater
        (ConcurrentLinkedQueue.class,  Node.class, "head");

    private boolean casTail(Node<E> cmp, Node<E> val) {
        return tailUpdater.compareAndSet(this, cmp, val);
    }

    private boolean casHead(Node<E> cmp, Node<E> val) {
        return headUpdater.compareAndSet(this, cmp, val);
    }

    private transient volatile Node<E> head = new Node<E>(null, null);

    private transient volatile Node<E> tail = head;

    public ConcurrentLinkedQueue() {}

    public ConcurrentLinkedQueue(Collection<? extends E> c) {
        for (Iterator<? extends E> it = c.iterator(); it.hasNext();)
            add(it.next());
    }

    public boolean add(E e) {
        return offer(e);
    }

    // 以下メソッド省略

こういったコードが念頭にあったため、定義→使用の順という発言になったのですが、よくよく考えてみると普段これとは逆の書き方もしている事に気付きました。

というのは主にメソッド定義です。リファクタリングでメソッドの抽出を行う場合、新しいメソッドは抽出元よりも下に加えています。

■Before
    public T get() {
        try {
            Future<T> result = cache.get();
            if (result != null) return result.get();
            FutureTask<T> task = new FutureTask<T>(originalFactory);
            if (cache.compareAndSet(null, task)) {
                task.run();
            }
            return cache.get().get();
        } catch (ExecutionException e) {
            throw new RuntimeException(e.getCause());
        } catch (InterruptedException e) {
            throw new CancellationException(e.getMessage());
        }
    }

■After
    public T get() {
        try {
            return getFuture().get();
        } catch (ExecutionException e) {
            throw new RuntimeException(e.getCause());
        } catch (InterruptedException e) {
            throw new CancellationException(e.getMessage());
        }
    }

    private Future<T> getFuture() {
        Future<T> result = cache.get();
        if (result != null) return result;
        FutureTask<T> task = new FutureTask<T>(originalFactory);
        if (cache.compareAndSet(null, task)) {
            task.run();
        }
        return cache.get();
    }

定義→使用とは逆ですね。

これはメソッドの場合、重要な情報はシグネチャに全て含まれる、つまり使用箇所を見れば大体どんなメソッドなのか見当がつく(つかなかったら名付けか何かが悪い)のに対し、クラスがどんなクラスなのか見当つけるには、そのクラスがどんな振る舞いを持つのかといった情報が必要になるからなのではないかと思います。

また、上記とは関係なく内部クラスの定義を下に書いている場合がありました。

というのはインスタンスを別表現にするようなアダプタ(例えばIteratorなど)やクラスの内部構造にあまり大きな影響を与えないコンテナを書く場合ですね。

import java.util.Calendar;
import java.util.Date;
import java.util.Iterator;
import java.util.NoSuchElementException;

public class Timespan implements Iterable<Date> {

    private final long start;
    private final long end;

    public Timespan(final Date start, final Date end) {
        this(start.getTime(), end.getTime());
    }

    public Timespan(final long start, final long end) {
        if (start >= end) throw new IllegalArgumentException("start >= end");
        this.start = start;
        this.end = end;
    }

    // (中略)

    @Override
    public Iterator<Date> iterator() {
        return iterator(TimeUnit.DAY, 1);
    }
    
    public Iterator<Date> iterator(final TimeUnit timeUnit, final int amount) {
        return new IteratorImpl(timeUnit, amount);
    }

    private class IteratorImpl implements Iterator<Date> {
        private final Calendar current;
        private final TimeUnit timeUnit;
        private final int amount;

        IteratorImpl(final TimeUnit timeUnit, final int amount) {
            this.timeUnit = timeUnit;
            this.amount = amount;
            current = Calendar.getInstance();
            current.setTime(new Date(start));
        }

        @Override
        public Date next() {
            if (!hasNext()) throw new NoSuchElementException();
            final Date result = current.getTime();
            current.add(timeUnit.getCalendarTypeValue(), amount);
            return result;
        }

        @Override
        public boolean hasNext() {
            return current.getTimeInMillis() < end;
        }

        @Override
        public void remove() {
            throw new UnsupportedOperationException();
        }
        
    }
}

結局ケースバイケースで選択していくという感じですかね。