##Getter、Setterを逆にするのは何故か
普通にコーディングに問題無い人でも、Getter、Setterを逆にするコードを見かける。取得すべきデータを戻り値としていればGetter、内部変数を変更したり、引数を更新する場合はSetterを使用すれば間違うことは無いと思うのだが、そう単純でもないのかもしれない。サンプルコードをみて考えてみる。

##サンプルコード
サンプルコードはいずれも、実際のプロジェクトで見たことがあるコードを簡略化している。
なお、NG1～NG3までの処理に関連はなく、一連の処理ではない。

        private void Main()
        {
            // NG1 戻り値がなく、引数を更新しているのにGetメソッド
            var dtoes = new List<Dto>();
            GetData(dtoes);

            // NG2 戻り値があるのにSetメソッド
            dtoes = SetData();

            // NG3 正しいSetterが混在していて読みにくい
            var dto2 = SetDto2();
            dto2.SetDtoes(SetData());
        }

        private void GetData(List<Dto> dtoes)
        {
            // DBから取得する想定。
            dtoes.Add(new Dto());
        }

        private List<Dto> SetData()
        {
            // DBから取得する想定。
            var dtoes = new List<Dto>();
            return new List<Dto>();
        }

        private Dto2 SetDto2()
        {
            var dto2 = new Dto2();
            return dto2;
        }

        /** テーブルのDTO **/
        private class Dto
        {

        }

        /** Dtoをメンバーに持つ親DTO **/
        private class Dto2
        {
            public List<Dto> _dtoes;

            public void SetDtoes(List<Dto> dtoes)
            {
                _dtoes = dtoes;
            }
        }

java

    public void main() {
        // NG1 戻り値がなく、引数を更新しているのにGetメソッド
        List<Dto> dtoes = new ArrayList<Dto>();
        getData(dtoes);

        // NG2 戻り値があるのにSetメソッド
        dtoes = setData();

        // NG3 正しいSetterが混在していて読みにくい
        Dto2 dto2 = setDto2();
        dto2.setDtoes(setData());
    }

    private void getData(List<Dto> dtoes) {
        // DBから取得する想定。
        dtoes.add(new Dto());
    }

    private List<Dto> setData() {
        // DBから取得する想定。
        List<Dto> dtoes = new ArrayList<Dto>();
        return new ArrayList<Dto>();
    }

    private Dto2 setDto2() {
        Dto2 dto2 = new Dto2();
        return dto2;
    }

    /** テーブルのDTO **/
    private class Dto {

    }

    /** Dtoをメンバーに持つ親DTO **/
    private class Dto2 {
        public List<Dto> _dtoes;

        public void setDtoes(List<Dto> dtoes) {
            _dtoes = dtoes;
        }
    }

##考察
上記サンプルコードのようになった理由を考えてみる。
GetData:DBからデータを取得しているのでGetter
SetData:DBから取得したデータを設定して返しているのでSetter
SetDto2:インスタンスを設定して返しているのでSetter
SetDtoes:Dtoを設定しているのでSetter
というところだろうか。
統一感がなくで、コードが読みにくい。
取得するデータを戻り値とするならGet, 内部変数の更新や引数の更新をSetというルールで書き直してみた。

##リファクタリング後

       private void Main()
        {
            // OK 引数更新
            var dtoes = new List<Dto>();
            SetData(dtoes);

            // OK 戻り値が取得データになっている。
            dtoes = GetData();

            // OK GetterとSetterが統一されて読みやすくなった。
            var dto2 = GetDto2();
            dto2.SetDtoes(GetData());
        }

        private void SetData(List<Dto> dtoes)
        {
            // DBから取得する想定。
            dtoes.Add(new Dto());
        }

        private List<Dto> GetData()
        {
            // DBから取得する想定。
            var dtoes = new List<Dto>();
            return new List<Dto>();
        }

        private Dto2 GetDto2()
        {
            var dto2 = new Dto2();
            return new Dto2();
        }

        /** テーブルのDTO **/
        private class Dto
        {

        }

        /** Dtoをメンバーに持つ親DTO **/
        private class Dto2
        {
            public List<Dto> _dtoes;

            public void SetDtoes(List<Dto> dtoes)
            {
                _dtoes = dtoes;
            }
        }

java

    public void main() {

        // OK 引数更新
        List<Dto> dtoes = new ArrayList<Dto>();
        setData(dtoes);

        // OK 戻り値が取得データになっている。
        dtoes = getData();

        // OK GetterとSetterが統一されて読みやすくなった。
        Dto2 dto2 = getDto2();
        dto2.setDtoes(getData());
    }

    private void setData(List<Dto> dtoes) {
        // DBから取得する想定。
        dtoes.add(new Dto());
    }

    private List<Dto> getData() {
        // DBから取得する想定。
        List<Dto> dtoes = new ArrayList<Dto>();
        return new ArrayList<Dto>();
    }

    private Dto2 getDto2() {
        Dto2 dto2 = new Dto2();
        return dto2;
    }

    /** テーブルのDTO **/
    private class Dto {

    }

    /** Dtoをメンバーに持つ親DTO **/
    private class Dto2 {
        public List<Dto> _dtoes;

        public void setDtoes(List<Dto> dtoes) {
            _dtoes = dtoes;
        }
    }

##まとめ GetなのかSetなのか迷ったら、取得するデータを戻り値とするか否かで決めればよい。

前の記事(関数呼び出しと条件分岐を分離できないか)

次の記事(なんでもかんでも配列)

目次