Java：コードをスッキリさせたい

Question

Javaのコードを整理して書きたいと考えています。
以下のようなコードを書き、動作できることは確認済みです。
ですが変数やif文が多すぎるので、もっとスッキリ書きたいのですが、
直す方法があれば教えていただけますか。

<やりたいこと>
DBからリスト形式でデータを取得し、該当するデータの数をカウントして
回数をテーブル(html)に表示させたい。

<データの例>
triangle white
circle red
circle blue
rhombus yellow
ellipse white
circle black
・ ・
・ ・
・ ・
・ ・

<テーブル>

---

• • • | circle | triangle | square | ellipse | rhombus | total

---

red | cnt_f1c1 | cnt_f2c1 | cnt_f3c1 | cnt_f4c1 | cnt_f5c1 | cnt_c1

---

blue | cnt_f1c2 | cnt_f2c2 | cnt_f3c2 | cnt_f4c2 | cnt_f5c2 | cnt_c2

---

yellow | cnt_f1c3 | cnt_f2c3 | cnt_f3c3 | cnt_f4c3 | cnt_f5c3 | cnt_c3

---

black | cnt_f1c4 | cnt_f2c4 | cnt_f3c4 | cnt_f4c4 | cnt_f5c4 | cnt_c4

---

white | cnt_f1c5 | cnt_f2c5 | cnt_f3c5 | cnt_f4c5 | cnt_f5c5 | cnt_c5

---

total | cnt_f1 | cnt_f2 | cnt_f3 | cnt_f4 | cnt_f5 | record

---

<ソースコード>
private String getDB(Model model){

List<Table1Entity> table1DataList = table1Repository.findAll();
int record = table1DataList.size();

int cnt_f1 = 0, cnt_f2 = 0, cnt_f3 = 0, cnt_f4 = 0, cnt_f5 = 0;
int cnt_c1 = 0, cnt_c2 = 0, cnt_c3 = 0, cnt_c4 = 0, cnt_c5 = 0;
int cnt_f1c1 = 0, cnt_f1c2 = 0, cnt_f1c3 = 0, f1c4 = 0, f1c5 = 0;
int cnt_f2c1 = 0, cnt_f2c2 = 0, cnt_f2c3 = 0, f2c4 = 0, f2c5 = 0;
int cnt_f3c1 = 0, cnt_f3c2 = 0, cnt_f3c3 = 0, f3c4 = 0, f3c5 = 0;
int cnt_f4c1 = 0, cnt_f4c2 = 0, cnt_f4c3 = 0, f4c4 = 0, f4c5 = 0;
int cnt_f5c1 = 0, cnt_f5c2 = 0, cnt_f5c3 = 0, f5c4 = 0, f5c5 = 0;

for (int i = 0; i < record; i++) {
	
String form = table1DataList.get(i).getForm();
String color = table1DataList.get(i).getColor();
	
if("circle".equals(form)) { 
		cnt_f1 += 1;
    	if("red".equals(color)) { cnt_f1c1 += 1; }
    	else if ("blue".equals(color)) { cnt_f1c2 += 1; }
    	else if ("yellow".equals(color)) { cnt_f1c3 += 1; }
    	else if ("black".equals(color)) { cnt_f1c4 += 1; }
    	else if ("white".equals(color)) { cnt_f1c5 += 1; }
	}
	else if ("triangle".equals(form)) {
		cnt_f2 += 1;
    	if("red".equals(color)) { cnt_f2c1 += 1; }
    	else if ("blue".equals(color)) { cnt_f2c2 += 1; }
    	else if ("yellow".equals(color)) { cnt_f2c3 += 1; }
    	else if ("black".equals(color)) { cnt_f2c4 += 1; }
    	else if ("white".equals(color)) { cnt_f2c5 += 1; }	
	}
	else if ("square".equals(form)) {
		cnt_f3 += 1;
    	if("red".equals(color)) { cnt_f3c1 += 1; }
    	else if ("blue".equals(color)) { cnt_f3c2 += 1; }
    	else if ("yellow".equals(color)) { cnt_f3c3 += 1; }
    	else if ("black".equals(color)) { cnt_f3c4 += 1; }
    	else if ("white".equals(color)) { cnt_f3c5 += 1; }		
	}
	else if ("ellipse".equals(form)) {
		cnt_f4 += 1;
    	if("red".equals(color)) { cnt_f4c1 += 1; }
    	else if ("blue".equals(color)) { cnt_f4c2 += 1; }
    	else if ("yellow".equals(color)) { cnt_f4c3 += 1; }
    	else if ("black".equals(color)) { cnt_f4c4 += 1; }
    	else if ("white".equals(color)) { cnt_f4c5 += 1; }		
	}
	else if ("rhombus".equals(form)) {
		cnt_f5 += 1;
    	if("red".equals(color)) { cnt_f5c1 += 1; }
    	else if ("blue".equals(color)) { cnt_f5c2 += 1; }
    	else if ("yellow".equals(color)) { cnt_f5c3 += 1; }
    	else if ("black".equals(color)) { cnt_f5c4 += 1; }
    	else if ("white".equals(color)) { cnt_f5c5 += 1; }		
	}
	
if("red".equals(color)) { cnt_c1 += 1; }
else if ("blue".equals(color)) { cnt_c2 += 1; }
else if ("yellow".equals(color)) { cnt_c3 += 1; }
else if ("black".equals(color)) { cnt_c4 += 1; }
else if ("white".equals(color)) { cnt_c5 += 1; }

}

model.addAttribute("cnt_f1", cnt_lb1);
model.addAttribute("cnt_f2", cnt_lb2);
model.addAttribute("cnt_f3", cnt_lb3);
model.addAttribute("cnt_f4", cnt_lb4);
model.addAttribute("cnt_f5", cnt_lb5);

model.addAttribute("cnt_f1c1", cnt_f1c1);
model.addAttribute("cnt_f1c2", cnt_f1c2);
model.addAttribute("cnt_f1c3", cnt_f1c3);
model.addAttribute("cnt_f1c4", cnt_f1c4);
model.addAttribute("cnt_f1c5", cnt_f1c5);

model.addAttribute("cnt_f2c1", cnt_f2c1);
model.addAttribute("cnt_f2c2", cnt_f2c2);
model.addAttribute("cnt_f2c3", cnt_f2c3);
model.addAttribute("cnt_f2c4", cnt_f2c4);
model.addAttribute("cnt_f2c5", cnt_f2c5);

model.addAttribute("cnt_f3c1", cnt_f3c1);
model.addAttribute("cnt_f3c2", cnt_f3c2);
model.addAttribute("cnt_f3c3", cnt_f3c3);
model.addAttribute("cnt_f3c4", cnt_f3c4);
model.addAttribute("cnt_f3c5", cnt_f3c5);

model.addAttribute("cnt_f4c1", cnt_f4c1);
model.addAttribute("cnt_f4c2", cnt_f4c2);
model.addAttribute("cnt_f4c3", cnt_f4c3);
model.addAttribute("cnt_f4c4", cnt_f4c4);
model.addAttribute("cnt_f4c5", cnt_f4c5);

model.addAttribute("cnt_f5c1", cnt_f5c1);
model.addAttribute("cnt_f5c2", cnt_f5c2);
model.addAttribute("cnt_f5c3", cnt_f5c3);
model.addAttribute("cnt_f5c4", cnt_f5c4);
model.addAttribute("cnt_f5c5", cnt_f5c5);

model.addAttribute("cnt_c1", cnt_c1);
model.addAttribute("cnt_c2", cnt_c2);
model.addAttribute("cnt_c3", cnt_c3);
model.addAttribute("cnt_c4", cnt_c4);
model.addAttribute("cnt_c5", cnt_c5);
model.addAttribute("cnt_all", record);

return result;

}

Comments

[jimbe]

> 動作できることは確認済み
コンパイルエラーになっています.

Answer 1

主題から少しずれてしまうかもしれませんが
処理が長くなっている原因は集計をJava側でやっているからというのもありますね
こういう集計はSQLに任せる方がそれっぽくまとまると思います

Answer 2

一番単純なのは配列を使うことです。各文字列から格納すべき場所を決定し、その場所のカウントを1増やす、という方法を繰り返します。

List<String> shapes = Arrays.asList("circle", "triangle", "square", "ellipse", "rhombus");
List<String> colors = Arrays.asList("red", "blue", "yellow", "black", "white");
int[][] counts = new int[colors.size() + 1][shapes.size() + 1];
for(Table1Entity entity : table1DataList) {
    int colomn = shapes.indexOf(entity.getForm());
    int row = colors.indexOf(entity.getColor());

    counts[row][column]++;
    counts[row][shapes.size()]++;
    counts[colors.size()][column]++;
}

対応が見えにくい、という場合は、Mapをネストして使うという方法も考えられます。

Answer 3

form X colorすべての組み合わせを表に集計

formの一覧、colorの一覧をプログラムで固定せず、データベースから読み込んだデータを利用します。
form X colorの組み合わせをすべて揃えるために、Table1Entityに度数(freq)フィールドを追加します。freqの値は次のように設定。

• 1 データベースから読み込んだデータ
• 0 初期値データ（ダミー）

public class Table1Entity {
  private String form;
  private String color;
  int freq; // 1 データベースから読み込み、 0 初期値として作成

form名のヘダーデータ（昇順）。プログラム定数を持たずデータベースのデータを利用。

// from ヘダー : 文字列昇順
Set<String> formHeader = table1DataList.stream()
    .map(Table1Entity::getForm)
    .collect(Collectors.toCollection(TreeSet::new));    
System.out.println(formHeader);

color名のヘダーデータ（昇順）。プログラム定数を持たずデータベースのデータを利用。

// color ヘダー : 文字列昇順
Set<String> colorHeader = table1DataList.stream()
    .map(Table1Entity::getColor)
    .collect(Collectors.toCollection(TreeSet::new));
System.out.println(colorHeader);

form X color の直積 (ダミー) 表。度数 0 の一覧。

// form X color の直積　（0カウンター）
Stream<Table1Entity> initialCounters = colorHeader.stream()
    .flatMap(e -> formHeader.stream()
        .map(f -> new Table1Entity(f, e, 0)));

直積 (ダミー) 表とデータベースのデータを併合して集計します。Table1Entity.freqを加算することで度数を求めます。

// 度数集計
Map<String,Map<String,Long>> freqs = 
  Stream.concat(table1DataList.stream(),initialCounters)
        .collect(Collectors.groupingBy(
            Table1Entity::getColor,
            TreeMap::new,
            Collectors.groupingBy(
                Table1Entity::getForm,
                TreeMap::new,
                Collectors.summingLong(Table1Entity::getFreq)) // 度数を加算
));
System.out.println(freqs);
System.out.println(freqs.values().stream().map(e -> e.values()).collect(Collectors.toList()));

度数集計を示すだけで、列集計、行集計はしていません。

Comments

[xebme]

すっきりだろうか？これはまだ冗長かもしれません。

Answer 4

スッキリの基準は人それぞれですので, どこまでやれば良いのかは判断できませんが.
ランダムに10件のデータを用意して集計しています.
一応 model.addAttribute まで行うようにしましたが, 無いデータ(=0件)分は行っていません.

import java.util.*;

public class Q214024 {
  private static final List<String> FORMS = Arrays.asList("circle","triangle","square","ellipse","rhombus");
  private static final List<String> COLORS = Arrays.asList("red","blue","yellow","black","white");
  private static final String KEY_SEPARATOR = ":";

  private String getAttributeName(String form, String color) {
    if(form.isEmpty() && color.isEmpty()) return "cnt_all";
    StringBuilder name = new StringBuilder("cnt_");
    if(!form.isEmpty() ) name.append("f").append(FORMS.indexOf(form)  +1);
    if(!color.isEmpty()) name.append("c").append(COLORS.indexOf(color)+1);
    return name.toString();
  }

  private String getDB(Model model){
    List<Table1Entity> table1DataList = table1Repository.findAll();

    //集計
    Map<String,Integer> map = new HashMap<>();
    for (Table1Entity entity : table1DataList) {
      String form = entity.getForm();
      String color = entity.getColor();
      for(String key : Arrays.asList(form+KEY_SEPARATOR+color,form+KEY_SEPARATOR,KEY_SEPARATOR+color)) {
        map.put(key, map.containsKey(key) ? map.get(key)+1 : 1);
      }
    }
    map.put(KEY_SEPARATOR, table1DataList.size());

    //設定
    for(Map.Entry<String, Integer> entry : map.entrySet()) {
      String[] keys = entry.getKey().split(KEY_SEPARATOR);
      String form  = keys.length >= 1 ? keys[0] : "";
      String color = keys.length >= 2 ? keys[1] : "";
      model.addAttribute(getAttributeName(form, color), entry.getValue());
    }

    return ""+table1DataList.size();
  }

  // 以下テスト用
  private Table1Repository table1Repository = new Table1Repository();
  void add(String form, String color) {
    table1Repository.add(form, color);
  }
  void printAll() {
    table1Repository.printAll();
  }
  static class Model {
    void addAttribute(String name, int value) {
      System.out.println("name="+name+", value="+value);
    }
  }
  static class Table1Entity {
    private String form, color;
    Table1Entity(String form, String color) { this.form=form; this.color=color; }
    String getForm(){ return form; }
    String getColor() { return color; }
    public String toString() { return form+":"+color; }
  }
  static class Table1Repository {
    private List<Table1Entity> list = new ArrayList<Table1Entity>();
    void add(String form, String color) {
      list.add(new Table1Entity(form, color));
    }
    List<Table1Entity> findAll() {
      return list;
    }
    void printAll() {
      for(Table1Entity entity : list) System.out.println(entity);
    }
  }

  public static void main(String args[]) {
    Q214024 q214024 = new Q214024();
    Random random = new Random();
    for(int i=0; i<10; i++) {
      q214024.add(FORMS.get(random.nextInt(FORMS.size())), COLORS.get(random.nextInt(COLORS.size())));
    }
    q214024.printAll();
    System.out.println("----");
    q214024.getDB(new Model());
  }
}

rhombus:white
ellipse:black
triangle:white
square:red
rhombus:white
ellipse:white
triangle:black
triangle:yellow
triangle:black
rhombus:white
----
name=cnt_f4, value=2
name=cnt_f3, value=1
name=cnt_f5, value=3
name=cnt_f2c5, value=1
name=cnt_f5c5, value=3
name=cnt_f2, value=4
name=cnt_c1, value=1
name=cnt_c3, value=1
name=cnt_f4c5, value=1
name=cnt_f2c3, value=1
name=cnt_f2c4, value=2
name=cnt_c4, value=3
name=cnt_f4c4, value=1
name=cnt_f3c1, value=1
name=cnt_c5, value=5
name=cnt_all, value=10

全ての Attribute を設定するため, 集計をクラス化してみました.
form/color を enum にし, 異常値を弾くようにしました.

import java.util.*;

public class Q214024_2 {

  enum Form {
    circle, triangle, square, ellipse, rhombus, total;
    static int size() { return Form.values().length; }
    //テスト用
    static Form getValue(Random random) {
      return Form.values()[random.nextInt(Form.total.ordinal())];
    }
  };
  enum Color {
    red, blue, yellow, black, white, total;
    static int size() { return Color.values().length; }
    //テスト用
    static Color getValue(Random random) {
      return Color.values()[random.nextInt(Color.total.ordinal())];
    }
  };

  //集計テーブル
  private static class SummaryTable {
    private int[][] values; //件数
    SummaryTable() {
      values = new int[Form.size()][Color.size()];
      for(int i=0; i<values.length; i++)
        for(int j=0; j<values[i].length; j++)
          values[i][j] = 0;
    }
    //form,colorの件数を +1 する
    void increment(Form form, Color color) {
      values[form.ordinal()][color.ordinal()] ++;
    }
    //(Model に設定する) Attribute と件数のマップを返す
    Map<String,Integer> getAttributeMap() {
      calcTotals();
      Map<String,Integer> map = new HashMap<>();
      for(Form form : Form.values()) {
        for(Color color : Color.values()) {
          map.put(getAttributeName(form,color), values[form.ordinal()][color.ordinal()]);
        }
      }
      return map;
    }
    //各合計を計算
    private void calcTotals() {
      for(int j=0; j<Color.size(); j++) values[Form.total.ordinal()][j] = 0;
      for(int i=0; i<Form.size(); i++) {
        values[i][Color.total.ordinal()] = 0;
        for(int j=0; j<Color.total.ordinal(); j++) {
          values[i][Color.total.ordinal()] += values[i][j];
          if(i != Form.total.ordinal()) values[Form.total.ordinal()][j] += values[i][j];
        }
      }
    }
    //Attribute 名を返す
    private String getAttributeName(Form form, Color color) {
      if(form == Form.total && color == Color.total) return "cnt_all";
      StringBuilder name = new StringBuilder("cnt_");
      if(form  != Form. total) name.append("f").append(form.ordinal() +1);
      if(color != Color.total) name.append("c").append(color.ordinal()+1);
      return name.toString();
    }
  }

  private String getDB(Model model){
    List<Table1Entity> table1DataList = table1Repository.findAll();

    //集計
    SummaryTable table = new SummaryTable();
    for (Table1Entity entity : table1DataList) {
      try {
        Form form = Form.valueOf(entity.getForm());
        Color color = Color.valueOf(entity.getColor());
        table.increment(form, color);
      } catch(IllegalArgumentException e) {
        System.err.println("form または color の値が異常です. form="+entity.getForm()+", color="+entity.getColor());
      }
    }

    //設定
    for(Map.Entry<String, Integer> entry : table.getAttributeMap().entrySet()) {
      model.addAttribute(entry.getKey(), entry.getValue());
    }

    return ""+table1DataList.size();
  }

  // 以下テスト用
  private Table1Repository table1Repository = new Table1Repository();
  void add(String form, String color) {
    table1Repository.add(form, color);
  }
  void printAll() {
    table1Repository.printAll();
  }
  static class Model {
    void addAttribute(String name, int value) {
      System.out.println("name="+name+", value="+value);
    }
  }
  static class Table1Entity {
    private String form, color;
    Table1Entity(String form, String color) { this.form=form; this.color=color; }
    String getForm(){ return form; }
    String getColor() { return color; }
    public String toString() { return form+":"+color; }
  }
  static class Table1Repository {
    private List<Table1Entity> list = new ArrayList<Table1Entity>();
    void add(String form, String color) {
      list.add(new Table1Entity(form, color));
    }
    List<Table1Entity> findAll() {
      return list;
    }
    void printAll() {
      for(Table1Entity entity : list) System.out.println(entity);
    }
  }

  public static void main(String args[]) {
    Q214024_2 q214024 = new Q214024_2();

    Random random = new Random();
    for(int i=0; i<10; i++) {
      q214024.add(Form.getValue(random).name(), Color.getValue(random).name());
    }
    //q214024.add("zzz", Color.getValue(random).name());
    q214024.printAll();

    System.out.println("----");
    q214024.getDB(new Model());
  }
}