Javaに関する様々な情報をご紹介します。

Javaに関する様々な情報をご紹介します。
評価

0

全体的に何が悪いのか

javaの勉強を始めて2日目になります。

以下を見て、全体的になにがダメなのか、
どう改善すればを大まかでいいので教えて頂けると幸いです。


//Movieクラス


package 面接用コード;
import static org.hamcrest.CoreMatchers.*;
import java.util.*;
import org.junit.*;
import play.test.*;
public class Movie {

            private String name;
            private String length;

            //席番号
            private String[] seats = new String[] {"1","2","3"};

            //埋まってる席番号
            private int[] setted = new int[3];
            public Movie() { }

        //nameをgetする
            public String getName() {
                return name;
            }

            //nameをセットする
            public void setName(final String movieName) {
                name = movieName;
            }

            //lengthをgetする
            public String getLength() {
                return length;
            }

            //説明付きlengthをgetする
            public String getLength2() {
                return "上映時間は" + length + "分です";
            }

            //lengthをsetする
            public void setLength(final String movieLength) {
                length = movieLength;
            }
            public int[] getSetted() {
                return setted;
            }
            public void setSetted(int[] setted) {
                this.setted = setted;
            }
            public String[] getSeats() {
                return seats;
            }
            public void setSeats(String[] seats) {
                this.seats = seats;
            }
}


//MovieManagerクラス

package 面接用コード;
import static org.hamcrest.CoreMatchers.*;
import java.util.*;
import org.junit.*;
import play.test.*;
public class MovieManager {

        //映画
        private Movie movie;

        //席番号
        private String[] seats;

        //埋まってる席番号
        private int[] setted;
        public MovieManager(Movie movie, String[] seats, int[] setted) {
            this.movie = movie;
            this.seats = seats;
            this.setted = setted;
        }

        public void getEmptySeatShow() {
            for(int i=0; i<seats.length; i++) {
               String num = seats[i];
               Integer tmp = setted[i];
               System.out.println("席番号" + num + "番は" + (tmp != null && tmp == 1 ? "埋まって" : "空いて") + "います");
            }
        }

        public void setSeat(int num) {
            setted[num - 1] = 1;
            movie.setSetted(setted);
        }
}



//Movieクラスのテストケース

package 面接用コード;
import static org.hamcrest.CoreMatchers.*;
import java.util.*;
import org.junit.*;
import play.test.*;
public class MovieTest extends UnitTest {

    //映画一覧
    ArrayList ml = null;

    @Before
    public void before() {

        //[システム] 映画を登録
       Movie movie01 = new Movie();
        movie01.setName("銀の弾丸などない"); movie01.setLength("120");
        Movie movie02 = new Movie();
         movie02.setName("スーツ対ギーク"); movie02.setLength("90");

         //[システム] 映画一覧
          ml = new ArrayList<Movie>();
         ml.add(movie01);
         ml.add(movie02);

    }

    @Test
    public void 映画座席予約() throws Exception {

        //[ユーザ] 映画一覧から選ぶ
         //時間が無いので、上映時間が100分以内のもの)
         Movie match = null;
         for(int i = 0; i<ml.size(); i++) {
             try {
               Movie tmp = (Movie) ml.get(i);
               String tmpLstr = tmp.getLength();
               Integer tmpL = new Integer(tmpLstr);
               if(100 < tmpL) {
                    match =tmp;
               }
             }catch(IndexOutOfBoundsException e) {}
         }

         //[システム] 選択した映画名を表示
         System.out.println("選択した映画:" + match.getName());
             // => 選択した映画:銀の弾丸などない

         //[システム] 空き座席を表示
         MovieManager mng = new MovieManager(match, match.getSeats(),match.getSetted());
         mng.getEmptySeatShow();
             // => 席番号1番は空いています
             // => 席番号2番は空いています
             // => 席番号3番は空いています

         //[ユーザ] 空き座席を選択
         mng.setSeat(2);

         //[システム] 空き座席を表示
         mng.getEmptySeatShow();
             // => 席番号1番は空いています
             // => 席番号2番は埋まっています
             // => 席番号3番は空いています
    }
}

3

回答

453

閲覧

3件の回答

評価

0

「全体的に」というとぼやけすぎているな…大きなものは

・MovieとMovieManagerの、役割分担がいい加減
・intとIntegerの区別がついていない
・lengthがStringなのはおかしい
・例えばsetSeat(0)で容易に例外を発生させることができる 引数は面倒でもチェックを入れる

あたりだろうか。
その他気になるところを挙げると

・~Managerは通常1つのオブジェクトに縛られない 1つであればそのままオブジェクトを操作するか、ラッパーになる
・フィールドで必須のものはsetterではなくコンストラクタで渡す
・getLength2()の内容のようなものは、呼び出し元で装飾する
・getEmptySeatShow()という名前はまったく意味が通らない メソッド名は動詞+名詞が基本
・映画座席予約()のループは全て100分以内なら全て代入が発生する
・setSeat(int num)でmovie.setSetted(setted);としているが無意味
・catch(IndexOutOfBoundsException e)としているが恐らく意味が分かっていないのでは
・finalとしているところがあるが統一感がない
・メソッド名に全角文字は使わないほうがいい
・settedという英単語は誤り

評価

0

・lengthはmovieLenghの方がわかりやすい
 (lengthという言葉自体Javaでは頻出するので、後々「どれの長さ(length)の変数だっけ?」となりやすい)
・席番号と埋まってる席番号の変数の型が違う
 (ほぼ同じ意味であれば、同じ型の方が混乱しなくて済む)

席番号と埋まってる席番号はMapあたりを使った方が楽な気がします。
(キーが席番号、値は埋まってるなら1、埋まってなければ0、みたいな感じ)

評価

0

まあmovieLengthでも良いが…より適切な表現をするならrunningTimeが良いと思う。

>席番号と埋まってる席番号はMapあたりを使った方が楽な気がします。
・文字列その他
・大きな数字
・飛び飛びの数字
のようなキーであればMapが適しているが、今回のような確定範囲の数列であれば配列で良い。

回答する

ログインしていません。

ログインしなくても回答はできますが、ログインすると、質問・回答の管理、更新があった場合のメールでの通知を受けることができます。 アカウントをお持ちでない方は会員登録を行ってください。

ユーザ名匿名