メソッドを使いよりすっきりとまとめたい。

Question

Java初心者です。現在格子状に値を配置してそれぞれの値を増加させるというプログラムをかなり雑になってしまいましたがこのプログラム自身は書きました。しかし、あまり見た目がよくありません。

　なのでメソッドを使いもう少し整理したいのですがメソッドの使い方に不慣れでうまくまとめることができませんでした。そもそもこれ以上まとめることができるのか。意見をいただけると幸いです。

class STAR23 {
      public static void main(String args[]){
        final int REGION_SIZE = 20;
        
        int n,i,j;
        int n_in_region[][] = new int[REGION_SIZE][REGION_SIZE];
      
        for( i = 0; i < REGION_SIZE; i++ ) {
          for( j = 0; j < REGION_SIZE; j++ ) {
            n_in_region[i][j] = 0;
          }
        }
        
        for( n = 0; n < 1000; n++ ){
          for( i = 0; i < REGION_SIZE; i++ ) {
            for( j = 0; j < REGION_SIZE; j++ ) {
              n_in_region[i][j] = one_percent_increase(n_in_region[i][j]);
              if( i > 0 ) {
                n_in_region[i][j] +=
                  one_percent_increase(n_in_region[i-1][j])
                    - n_in_region[i-1][j];
              }
              if( i < REGION_SIZE-1 ) {
                n_in_region[i][j] +=
                  one_percent_increase(n_in_region[i+1][j])
                    - n_in_region[i+1][j];
              }
            }
          }
          System.out.print("day " + n + ":\n");
          for( i = 0; i < REGION_SIZE; i++ ) {
            for( j = 0; j < REGION_SIZE; j++ ) {
              System.out.printf("%3d ",n_in_region[i][j]);
            }
            System.out.print("\n");
          }
        }
        
        return;
      }
      
      static int one_percent_increase(int num) {
        int r;
     
        r = num;
        r *= 1.01;
        if( r < 100 && (int)(Math.random() * 100) < 1 ) {
          r += 1;
        }
        return r;
      }
    }

Comments

[dice142]

まずコードをコードブロック（```）で囲んでください。

[shou_hitotose]

すみません。質問に不慣れなもので。ご指摘ありがとうございます

[y_waiwai]

質問文は編集できますんで、ソースコードは<code>ボタンを押して、’’’の枠の中に貼り付けましょう

[dice142]

できてないです。「`」３つをコードの前後に入れてください。あるいはヘルプ参照してください。

[shou_hitotose]

すみません。ちょっとお待ちください。

[shou_hitotose]

こういった形でよろしいでしょうか

[dice142]

大丈夫です。ありがとうございます。

[dice142]

「上手く実行できませんでした」とありますが、質問は「理想の動作をしない原因を知りたい」のか「コードをすっきりさせたい」のかどちらでしょう？

[退会済みユーザー]

プログラミング関係なしに、どういったことを行っているかメタいレベルでのコメントを入れましょう。回答者(レビュアー)に対して、「何をやっているかはコードから読み解いて下さい」という姿勢はちょっとどうかと。
例えば、「// 二次元配列を0で初期化します」など（ここは見れば一目瞭然ではありますが）。また、そうすることで質問者さん自身も見えてくるものがあるかと。（反論、ご指摘があれば歓迎!!）

[shou_hitotose]

ああ、すみません。

[shou_hitotose]

コードをすっきりさせたいの方です。

[shou_hitotose]

ppnさん、dice142さん、ｙ_waiwaiさんいろいろとご指導いただきありがとうございます。あらためてお礼申し上げます。

[swordone]

one_percent_increase内のrがdoubleになるとかじゃないとコンパイルエラーでは？

Answer 1

何をしているかまで見てないので、大まかに列挙します。

全体的に

• Javaの命名規則になるべく従う。

定数以外はキャメルケースを使うなど。

• for文のカウンタ変数の不要な宣言はしない。

for文の初期化の際に変数宣言も同時にできます。

• ネストが深くなるようであればcontinueのためのif文に変える

if (i == 0) continue;
n_in_region[i][j] += ....
	- n_in_region[i-1][j];

• 適宜コメントを入れる
• 適宜空行を挟む

メソッドを使うなら

• 処理ごとにまとめる

初期化、計算処理部、表示部など

Comments

[shou_hitotose]

いろいろと面倒なところが多いにもかかわらず回答までしていただきありがとうございます。とりあえずやってみようと思います！

[shou_hitotose]

回答ありがとうございました。また機会があればよろしくおねがいします。

Answer 2

元のソースコードを残すようにしてリファクタリングしてみました。
ロジック部分をプライベートメソッドに切り出しているだけですが。
まずはここからでどうでしょうか？

package moby.go.test;

public class STAR24 {

  private static int REGION_SIZE = 20;


  public static void main(String args[]){

    int n_in_region[][] = new int[REGION_SIZE][REGION_SIZE];

    initializeRegion(n_in_region);

    for(int n = 0; n < 1000; n++ ){
      calcurateRegion(n_in_region);
      display(n_in_region, n);
    }

    return;
  }


  static int one_percent_increase(int num) {
    int r;

    r = num;
    r *= 1.01;
    if( r < 100 && (int)(Math.random() * 100) < 1 ) {
      r += 1;
    }
    return r;
  }

  private static void initializeRegion(int[][] n_in_region) {
    for(int i = 0; i < REGION_SIZE; i++ ) {
      for(int j = 0; j < REGION_SIZE; j++ ) {
        n_in_region[i][j] = 0;
      }
    }
  }


  private static void calcurateRegion(int[][] n_in_region) {
    for(int i = 0; i < REGION_SIZE; i++ ) {
      for(int j = 0; j < REGION_SIZE; j++ ) {
        n_in_region[i][j] = one_percent_increase(n_in_region[i][j]);
        if( i > 0 ) {
          n_in_region[i][j] +=
            one_percent_increase(n_in_region[i-1][j])
              - n_in_region[i-1][j];
        }
        if( i < REGION_SIZE-1 ) {
          n_in_region[i][j] +=
            one_percent_increase(n_in_region[i+1][j])
              - n_in_region[i+1][j];
        }
      }
    }
  }


  private static void display(int[][] n_in_region, int day) {
    System.out.print("day " + day + ":\n");
    for(int i = 0; i < REGION_SIZE; i++ ) {
      for(int j = 0; j < REGION_SIZE; j++ ) {
        System.out.printf("%3d ",n_in_region[i][j]);
      }
      System.out.print("\n");
    }
  }

}

Answer 3

名前の付け方とかクラス作るとかは考えず、
元コードはあまり変えずメソッドの切り分けだけやってみました。

class Main {
    
    final static int REGION_SIZE = 20;
    
    public static void main(String args[]){

        int n_in_region[][] = new int[REGION_SIZE][REGION_SIZE];
        
        init_regions(n_in_region);

        for(int day = 0; day < 1000; day++){
            increase_regions(n_in_region);

            print_day_header(day);
            dump_regions(n_in_region);
        }

    }
      
    static void init_regions(int region[][]) {
        for (int i = 0; i < REGION_SIZE; i++) {
            for (int j = 0; j < REGION_SIZE; j++) {
                region[i][j] = 0;
            }
        }
    }
    
    static void print_day_header(int day) {
        System.out.print("day " + day + ":\n");
    }
    
    static void dump_regions(int region[][]) {
        for (int i = 0; i < REGION_SIZE; i++) {
            for (int j = 0; j < REGION_SIZE; j++) {
                System.out.printf("%3d ", region[i][j]);
            }
            System.out.print("\n");
        }
    }
    
    static void increase_regions(int region[][]) {
        for (int i = 0; i < REGION_SIZE; i++) {
            for (int j = 0; j < REGION_SIZE; j++) {
                region[i][j] = one_percent_increase(region[i][j]);
                if (i > 0) {
                    region[i][j] += one_percent_increase(region[i-1][j]) - region[i-1][j];
                }
                if (i < REGION_SIZE-1) {
                    region[i][j] += one_percent_increase(region[i+1][j]) - region[i+1][j];
                }
            }
        }
    }

    static int one_percent_increase(int num) {
        int r;

        r = num;
        r *= 1.01;
        if( r < 100 && (int)(Math.random() * 100) < 1 ) {
            r += 1;
        }
        return r;
    }
}

paiza.io で試してます。（1000回ループするとtimeoutしちゃいますが）

Comments

[shou_hitotose]

ご回答いただきありがとうございます！！

[shou_hitotose]

無事問題をかいけつすることが出来ました！

Answer 4

もう少し、分けてみました。

package moby.go.test;

import java.util.Arrays;

public class STAR25 {

  private static int REGION_SIZE = 20;

  public static void main(String[] args){
    int[][] region = new int[REGION_SIZE][REGION_SIZE];

    initializeRegion(region);

    for (int n = 0; n < 1000; ++n) {
      calcurateRegion(region);
      display(region, n);
    }

    System.exit(0);
  }


  private static void initializeRegion(int[][] region) {
    for (int elementNumber = 0; elementNumber < REGION_SIZE; ++elementNumber)
      Arrays.fill(region[elementNumber], 0);
  }


  private static void calcurateRegion(final int[][] region) {
    for (int i = 0; i < REGION_SIZE; ++i) {
      for(int j = 0; j < REGION_SIZE; ++j) {
        region[i][j] = increaseOnePercent(region[i][j]);
        if (i > 0 && (i < (REGION_SIZE - 1))) {
          region[i][j] = reCalcurateRegion(region[i][j], region[i-1][j]);
          region[i][j] = reCalcurateRegion(region[i][j], region[i+1][j]);
        }
      }
    }
  }


  private static void display(final int[][] region, final int day) {
    System.out.print("day " + day + ":\n");
    for (int i = 0; i < REGION_SIZE; ++i) {
      for (int j = 0; j < REGION_SIZE; ++j) {
        System.out.printf("%3d ", region[i][j]);
      }
      System.out.print("\n");
    }
  }


  private static int increaseOnePercent(final int number) {
    return correct((int)(number * 1.01));
  }


  private static int correct(final int number) {
    int result = number;
    return  (result < 100 && (Math.random() * 100) < 1)? result+=1 : result;
  }


  private static int reCalcurateRegion(final int source, final int number) {
    return source + increaseOnePercent(number) - number;
  }


}

Comments

[madoka9393]

回答内容には全く関係ありませんが、新たに回答を投稿するのではなく元の回答を編集または追記すればよかったのでは…。

[MAKOTO_MOBY]

おっしゃるとおりです。
申し訳ございません。
以後からそのように致します。

[退会済みユーザー]

私の場合、細かく分けてくか、コメント入れるか、悩むときがちょこちょこあるんですが、判断基準って何がよいのでしょうね。。。分けたメソッドに定義が散らばるの嫌なときは、分けなかったりするんですが、その判断基準ってどうなんだろうなぁ。と。（何が言いたいかよくわからないコメント）

[shou_hitotose]

再度回答いただきありがとうございます！大変助かりました！

[shou_hitotose]

ベストアンサーにしていただきました！また機会があればよろしくおねがいします