プログラムを書き換えたい

たびたび失礼します。
教科書の問題を解いていたのですが、他の書き方でも練習したいと思いました。
前の単元でローカル変数、メンバ変数などの違いを学んだので、それに基づいて
やろうと思ったのですが、思った以上に自分がメンバ変数に頼った書き方をしており、
どのように直していけばよいのか困っております。
（如何せん解答もありませんので…。）

以下自分の書いたプログラムで、コンパイル・実行ともにできます。
方針、アドバイスなどでも構いませんので宜しくお願いします。

##前提・実現したいこと
以下のプログラムをメンバ変数を使わないように書き換えたいです。

public class Mondai {

	public static void main(String[] args) {

		Countries con = new Countries();
		con.setNumber(3);

	}
}

class Countries {

	private int num = 0;//メンバ変数
	private String country = "";//メンバ変数

	protected void setNumber( int suuji ){

		for(int i = 0; i < suuji ;i++){
			num = i;
			sayCountry();
			System.out.println( country );
		}

	}

	private void sayCountry(){

		switch(num){
			case 0:
				country = "アメリカ";
				break;
			case 1:
				country = "ロシア";
				break;
			case 2:
				country = "中国";
				break;
			case 3:
				country = "フランス";
				break;
			case 4:
				country = "イギリス";
				break;
			case 5:
				country = "イタリア";
				break;
			default:
				break;
/*出力結果
アメリカ
ロシア
中国
*/
		}
	}
}

##補足情報(言語/FW/ツール等のバージョンなど)
Java テキストエディタ（Windows）、コマンドプロンプト

行動規範の内容に同意します

回答4件

個人的な感想です。

１）sayCountryメソッドの挙動が、引数ではなくメンバー変数　 num によって決まるのは気持ち悪い。また、実行結果がメンバー変数 countryで暗に受け渡されるのも同様。（コードを全部読まないと判らないような関係は、バグの元）

また、numと国名の関連付けは、配列で定義するほうが判り易い。(Switch-Case構文に目を通さないといけないのは苦痛）

引数で国に対応する数値を与えて、国の名前を返すようなメソッド（例えば、以下のコードのようなもの）にすれば、値の受け渡しが明瞭になる。

private String countryNameByNumber(int num){
  private String countryNames[] = {"アメリカ","ロシア","中国","フランス","イギリス","イタリア"};

  if (num >= 6) {
    // エラーメッセージを出す
  } else {
    return countryNames[num];
  }
}

２）メソッド名が実態を表していない。

上の例では、引数が番号で、返り値が国名のメソッドなので、countryNameByNumber（数字による国名）というメソッド名にしました。

　質問のコードを見てみると
"sayCountry"は何を表しているだろうか？国を言う、って何をするメソッドなんだろう。
"setNumber"は数字をセットするメソッド？　実態(コード)は引数の数だけ国名を表示するようになっている。

　こうゆう疑問を持たれるようなコードは避けるべきです。
コードを書いたときは、何なのか判ります（覚えています）。でも、２，３か月後に見たら、自分が書いたコードにむかって「ダレが書いたんだ！」と怒鳴っているかもれしれませんよ。

投稿2018/05/16 08:59

coco_bauer

総合スコア6915

sky.user.

2018/05/17 00:27

ご回答ありがとうございます。まだコーディング規約などを扱ったことがなく、お見苦しい命名、失礼しました。あと今までif文の中でエラーメッセージを返すものを見たことがなかったので、とても新鮮でした（いつもelseのあとで出していたので...。）。考えてみたら当たり前なのですが、そのような書き方もできるのですよね。勉強になりました。 2,3ヵ月後、そのように思えるようになるように頑張ります（笑）具体的なコードまで示してくださり、ありがとうございました！

行動規範の内容に同意します

java
1public class Mondai {
2
3    public static void main(String[] args) {
4
5        Countries con = new Countries();
6        con.setNumber(3);
7
8    }
9}
10
11class Countries {
12
13    protected void setNumber( int suuji ){
14
15        string country;
16
17        for(int i = 0; i < suuji ;i++){
18            country = sayCountry(i);
19            System.out.println( country );
20        }
21
22    }
23
24    private string sayCountry(int num){
25
26        string country="none";
27        switch(num){
28            case 0:
29                country = "アメリカ";
30                break;
31            case 1:
32                country = "ロシア";
33                break;
34            case 2:
35                country = "中国";
36                break;
37            case 3:
38                country = "フランス";
39                break;
40            case 4:
41                country = "イギリス";
42                break;
43            case 5:
44                country = "イタリア";
45                break;
46            default:
47                break;
48/*出力結果
49アメリカ
50ロシア
51中国
52*/
53        }
54        return country;
55    }
56}
57

あえて質問文に合わせました

投稿2018/05/16 08:04

y_waiwai

総合スコア87774

sky.user.

2018/05/16 08:08

ご丁寧にありがとうございます！

行動規範の内容に同意します

こんにちは、

String []country_name = {"アメリカ","ロシア","中国","フランス","イギリス","イタリア"};

のような配列を準備するとswitch文を書かずに済みます。
ただし、要素数以上の数字が入るとエラーが出るので要素数で割ったときの余りに変換しています。

Java
1public class Main {
2    public static void main(String[] args) {
3
4        Countries con = new Countries();
5        con.setNumber(3);
6    }
7}
8
9class Countries {
10
11    private int num;//メンバ変数
12    private String country;//メンバ変数
13
14    protected void setNumber(int suuji){
15
16        for(int i = 0; i < suuji ;i++){
17            num = i;
18            sayCountry();
19            System.out.println(country);
20        }
21    }
22
23    private void sayCountry(){
24
25       String []country_name = {"アメリカ","ロシア","中国","フランス","イギリス","イタリア"};
26       country = country_name[num % country_name.length];
27    }
28}

また、クラスを使わずメソッドだけで書く方法もあります。
ここも、要素数以上の数字が入るとエラーが出るので要素数で割ったときの余りに変換しています。

Java
1public class Main {
2  public static void main(String[] args) {
3        
4      print_country(3);
5        
6  }
7  public static void print_country(int n){
8        
9    String []country_name = {"アメリカ","ロシア","中国","フランス","イギリス","イタリア"};
10    n = n % country_name.length;
11    for(int i = 0; i < n; i++){
12      System.out.println(country_name[i]);
13    }
14  }
15}

投稿2018/05/16 08:34

退会済みユーザー

総合スコア0

sky.user.

2018/05/16 08:59

ありがとうございます！初めてみるやり方です...！ >>要素数以上の数字が入るとエラーが出るので要素数で割ったときの余りに変換これは普通にエラーを例外としてtry-catchで逃がしても良いのでしょうか？

退会済みユーザー

2018/05/16 09:34

はい、 try{ }catch(ArrayIndexOutOfBoundsException e){} の形でできます。詳しくは http://www1.bbiq.jp/takeharu/java62.htmlをご覧ください。

退会済みユーザー

2018/05/16 09:36

switch文だとdefaultがあるので　このようなtry~catchが不要ですが、コードが長くなり、逆にswitch文を使わない方法(回答の配列)だとtry~catchが必要なのでどちらも長所、短所があります。 (私も最初switch文で書こうか迷いました。)

sky.user.

2018/05/17 00:30

URL、ありがとうございます。今までif文で書いていたため、今回はswitch文の練習として書いていたのですが、本当に色々な書き方ができるのですね...（その中でも読みやすかったり、処理が早かったりと優れたものがあるのでしょうが。）是非、また別の機会に皆さんの読みやすい（または読みにくい）と思うコードの共通点などを伺ってみたいです。

行動規範の内容に同意します

ベストアンサー

こんな感じで書けば、フィールドは不要ですね。

Java
1private void sayCountry(int num){
2    String country;
3
4    switch(num){
5    case 0:
6        country = "アメリカ";
7        break;
8    case 1:
9        country = "ロシア";
10        break;
11    case 2:
12        country = "中国";
13        break;
14    case 3:
15        country = "フランス";
16        break;
17    case 4:
18        country = "イギリス";
19        break;
20    case 5:
21        country = "イタリア";
22        break;
23    default:
24        return;
25    }
26
27    System.out.println(country);
28}