抽象クラスにおいて、引数は違うが機能はほぼ同じようなメソッドを持つクラスをどうまとめるべきか、教えてください

Question

前提・実現したいこと

抽象クラスについてわからないことがあったので回答いただけると助かります。プログラミングは独学ですが、楽しく学んでおります。

悩んでいることは、「引数が違うが、機能がほとんど同じメソッドはどのように記述すべきか」です。具体的には、いまカードゲームの作成に挑戦しています。カードが置かれる場所として「山札」「手札」「フィールド」「墓地(捨て札を置く)」があり、それぞれに共通した機能があります(カードを追加、カードを取り除く、その場所のカードを取得等)。ただ、カードの追加に関してはそれぞれ引数が違います。墓地にカードを置く場合、引数はカードのみで済みます。一方フィールドや山札の場合、引数はカードだけでは不十分で、どこに追加したいかも必要です。しかし処理自体は非常によく似ています。

試したこと

下に載せたコードでほしい機能は実現できます。ただ、墓地にカードを加えるときAddCard()の第二引数には意味のない整数を書く必要があり、もやもやします。この書き方は正しいのでしょうか。よりわかりやすい書き方があるのであれば、ぜひ教えていただきたいです。

public abstract class Position
{
    public abstract List<Card> Cards{get;}
    public virtual void AddCard(Card card, int index)
    {
        //共通の機能
    }
    public virtual bool RemoveCard(Card card)
    {
        //共通の機能
    }
}

teratailは初めての投稿なので、間違った使い方をしていたらご指摘をよろしくお願いします。

Answer 1

ゲームの仕様がよくわからないので単純に思った点ですが、そもそも「単純に追加」と「場所を指定して追加」をまとめようとする事自体に疑問があります。

これはつまり、位置指定が必須なものとそうでないものが、同じ基底クラスから派生すべきものなのかどうか、という疑いです。

前者が単純なAddメソッドであるのに対し、後者はAddTo(index)みたいなメソッドになるべきでしょう。

だとすると、これは本当に同じ基底クラスから派生するという設計になるんでしょうか？

「山札」「手札」「フィールド」「墓地(捨て札を置く)」というクラスを分析した場合、明らかにそこに相違点があるのに、is-a関係が成立するようには思えません。

これはクラスの機能にのみ着目して継承関係を作ろうとしたときによくある間違いのような気がします。

最初から継承関係ではなく、「持ち札」というコレクションクラスを包含するような個々の独立したクラスとして設計していれば、このような悩みはなかったのではないでしょうか。

Comments

[Zuishin]

全くその通りだと思いますが、他の人に伝わっていないかもしれません。

System.Collections.Generic.Collection<T> には Add(T) と Remove(T) があり、System.Collections.Generic.List<T> にはそれに加えて Insert(int, T) があります。
山札や手札は AddCard や InsertCard を実装する必要はなく、ただコレクションやリストをプロパティとして持てばいいというだけのことですね。

挿入や削除をした際についでに何か他のことをしたいなら、Collection<T> から派生したクラスをプロパティとして持てば何とでもなると思います。

コレクションの仕事はコレクションに独立させるべきで、全ての機能を一つのクラスが持つ必要はないし、持たせるのはアンチパターンだと思います。

もっとも、もしも山札や手札が単なるコレクションの域を出ないのであれば、それらを Collection<T> から派生させるのもありだと思いますが。

[gentaro]

補足説明ありがとうございます。
確かに、概念モデルの分析、設計フェーズの話として回答したため、ちょっと伝わりづらい内容だったかもしれません。

実装レベルの話であれば補足していただいた内容の通りですし、fenaさんの回答にあるように「基底型.Add」のような呼び出しをそもそも考慮していない（できない）はずなので、継承関係関係としておかしいのではないか？という事です。

[Su-Lu-St]

ありがとうございました。継承を使った実装が間違っていたということを(完全にではないですが)理解できた気がします。自分が正しく実装できているかを知ることはとても難しいですが、これからも精進しようと思います。回答をしていただいた皆様、どうもありがとうございます。

Answer 2

そもそも，

カードを追加、カードを取り除く、その場所のカードを取得等

の処理(e.g. 「墓地」や「山札」や「手札」のAddCard()メソッドを呼ぶ側の処理)というのは，
「墓地」とその他（山札とか）を，区別せずに扱う実装(すなわち，Position型への操作としての実装)が成されるのでしょうか？

●Yesであれば，「墓地」とその他は同一のインタフェースでなければ困るわけだし，「カードを追加」を行う場所では「対象が「墓地」なのか違うのか」を知らないハズなのだから，

墓地にカードを加えるときAddCard()の第二引数には意味のない整数を書く必要があり、もやもやします

なんて話にはならない．

●Noであれば，「墓地にカードを加える処理」というのは，「その他の対象にカードを加える処理」とは別の実装がされているということ（Position型.AddCard()を呼ぶのではなく，墓地型.AddCard()を呼ぶコードになっている）であり，
であれば，「墓地」とその他に共通のインタフェースを持たせる必要性は無い．
「墓地」とその他は，その内部で具体実装において，ある共通のコードを利用するかもしれないが，それだけの関係．
そういった「あるコードを使い回す手段」として「継承」を用いる必要はないし，用いるべきでもないと思う．

Answer 3

public abstract class Position
{
    public abstract List<Card> Cards{get;}
    public virtual void AddCard(Card card, int index)
    {
        //共通の機能
    }
    public virtual void AddCard(Card card)
    {
        AddCard(card,0);
    }
    public virtual bool RemoveCard(Card card)
    {
        //共通の機能
    }
}

みたいに、オーバーロードさせて、省略した場合はデフォルトの位置に置くみたいにするとか…？
(デフォルトの値が何かはわからないので適当です)
墓地の方でも、index付きが使えるけど、「その値は無視されます」という仕様にしておくとか。

Answer 4

位置を指定して追加、末尾に追加、これらは別の機能なので素直にメソッドを分ければ良いのではないでしょうか。
IList.InsertとIList.Addの違いです。

ただ、こういう単純な例ならいいのですが、メソッドの実行にもっと複雑な条件指定が必要な場合、個別に引数でメソッドやオーバーロードを用意すると死ねるので、渡すオブジェクト側にプロパティを持たせたり、条件指定用のプロパティを持ったクラスを引数にして渡したりします。