現在「Java言語で学ぶリファクタリング(結城 浩著)」を進めています。
本著の第6章練習問題6-2について質問させていただきます。
この問題と模範解答は実際のコードが記載しておらずクラス図しか記載してありません。
そのため以下に示すコードは私がクラス図から起こしたものです。(コードに起こす段階から勘違いしている可能性もありえます。)
問題は以下のとおりです。
Playerクラスは、音楽やビデオを再生するクラスです。setMediaメソッドで音楽かビデオを選択し、play,loop,stopの各メソッドで、それぞれ再生、ループ、停止の処理をおこないます。音楽とビデオの再生方法は異なるので、音楽に大してplayメソッドを実行する場合には内部的にplayMusicメソッドを呼び出し、ビデオの場合には内部的にplayVideoメソッドを呼びだします(loop,stopについても同様)。
このクラスの問題点を指摘し、改善案を示してください。
Java
1//問題があるクラス 2 3public class Player { 4 5 public static final int MUSIC = 1; 6 public static final int VIDEO = 2; 7 8 private int currentMedia; 9 private String musicData; 10 private String videoData; 11 12 public Player(String musicData, String videoData){ 13 this.musicData= musicData; 14 this.videoData= videoData; 15 } 16 17 public void setMedia(int currentMedia){ 18 this.currentMedia = currentMedia; 19 } 20 21 public void play(){ 22 if(currentMedia == MUSIC){ 23 playMusic(); 24 }else if(currentMedia == VIDEO) { 25 playVideo(); 26 } 27 } 28 29 public void loop(){ 30 if(currentMedia == MUSIC){ 31 loopMusic(); 32 }else if(currentMedia == VIDEO) { 33 loopVideo(); 34 } 35 } 36 37 public void stop(){ 38 if(currentMedia == MUSIC){ 39 stopMusic(); 40 }else if(currentMedia == VIDEO) { 41 stopVideo(); 42 } 43 } 44 45 private void stopVideo() { 46 System.out.println("stopVideo:" + videoData); 47 } 48 49 private void stopMusic() { 50 System.out.println("stopMusic:" + musicData); 51 } 52 53 private void loopVideo() { 54 System.out.println("loopVideo:" + videoData); 55 } 56 57 private void loopMusic() { 58 System.out.println("loopMusic:" + musicData); 59 } 60 61 private void playVideo() { 62 System.out.println("playVideo:" + videoData); 63 } 64 65 private void playMusic() { 66 System.out.println("playMusic:" + musicData); 67 } 68} 69
この問題に対する私の解答は以下のとおりです。
Java
1//改善後のクラス(私の解答) 2public interface Player { 3 4 public void play(); 5 6 public void loop(); 7 8 public void stop(); 9} 10 11class Media { 12 String data; 13 14 public Media(String data){ 15 this.data = data; 16 } 17 18 public String toString(){ 19 return data; 20 } 21} 22 23class MusicPlayer implements Player { 24 Media data; 25 public MusicPlayer(Media data){ 26 this.data = data; 27 } 28 29 @Override 30 public void play(){ 31 System.out.println("MusicPlayer:play:" + this.data); 32 } 33 34 @Override 35 public void loop(){ 36 System.out.println("MusicPlayer:loop:" + this.data); 37 } 38 39 @Override 40 public void stop(){ 41 System.out.println("MusicPlayer:stop:" + this.data); 42 } 43} 44 45class VideoPlayer implements Player { 46 Media data; 47 public VideoPlayer(Media data){ 48 this.data = data; 49 } 50 51 @Override 52 public void play(){ 53 System.out.println("VideoPlayer:play:" + this.data); 54 } 55 56 @Override 57 public void loop(){ 58 System.out.println("VideoPlayer:loop:" + this.data); 59 } 60 61 @Override 62 public void stop(){ 63 System.out.println("VideoPlayer:stop:" + this.data); 64 } 65}
play,stop,loopといったPlayerクラスの動作を表すメソッドに着目し、VideoPlayerクラスとMusicPlayerクラスの2種類が必要と考えました。
Mediaクラスはあくまでデータの塊であり、動作を表すメソッドは存在しません。
一方で、本著の模範解答は以下の通りでした。
Java
1//改善後のクラス(模範解答) 2public class Player { 3 Media media; 4 5 public void play(){ 6 System.out.println("Player:play"); 7 media.play(); 8 } 9 10 public void loop(){ 11 System.out.println("Player:loop"); 12 media.loop(); 13 } 14 15 public void stop(){ 16 System.out.println("Player:stop"); 17 media.stop(); 18 } 19 20 public void setMedia(Media media){ 21 this.media = media; 22 } 23} 24 25class Media { 26 String data; 27 28 public Media(String data){ 29 this.data = data; 30 } 31 32 public void play(){ 33 System.out.println("Media:play"); 34 } 35 36 public void loop(){ 37 System.out.println("Media:loop"); 38 } 39 40 public void stop(){ 41 System.out.println("Media:stop"); 42 } 43} 44 45class Music extends Media{ 46 47 public Music(String data){ 48 super(data); 49 } 50 51 @Override 52 public void play(){ 53 System.out.println("Music:play:" + this.data); 54 } 55 56 @Override 57 public void loop(){ 58 System.out.println("Music:loop:" + this.data); 59 } 60 61 @Override 62 public void stop(){ 63 System.out.println("Music:stop:" + this.data); 64 } 65} 66 67class Video extends Media{ 68 69 public Video(String data){ 70 super(data); 71 } 72 73 @Override 74 public void play(){ 75 System.out.println("Video:play:" + this.data); 76 } 77 78 @Override 79 public void loop(){ 80 System.out.println("Video:loop:" + this.data); 81 } 82 83 @Override 84 public void stop(){ 85 System.out.println("Video:stop:" + this.data); 86 } 87} 88
以上です。
どうも模範解答に納得できません。
MusicやVideoというデータを表すクラスがplay,stop,loopといった動作を表すメソッドをもっていることに違和感を感じてしまいます。
しかし、私の解答ですと、Mediaクラスのインスタンスと、どちらかのPlayerクラスのインスタンスの2つをつくらなければなりませんし、setMediaメソッドであとからPlayerの動作を変化させることはできないなど使い勝手が悪い気もします。
長くなりましたが、お聞きしたいことは以下です。
・私の解答に問題はないか?
・模範解答のように、MusicクラスとVideoクラスに動作を表すメソッドが存在してもよい理由はなにか?
よろしくお願いします。
回答5件
あなたの回答
tips
プレビュー