ほぼ同じ処理のメソッド化

Question

お世話になります。
メソッド化について質問があり、ご教授いただければと思います。

処理が全く同じものをメソッド化にすることはできるのですが、
少し、変数名や値が異なるもの処理をメソッド化する方法がわかりません。
以下ソース
public static void main(String[] args) throws IOException{

	String line = "";
	BufferedReader br = null;
	String dir = (args[0]);
	dir = dir + "\\";
	
	File branchFile = new File(dir + BRANCH);
	if (!branchFile.exists()) {
		System.out.println("店舗ファイルが存在しません");
		return;
	}
	List<Map<String, String>> branchs = new ArrayList<Map<String, String>>();
	try {
		br = new BufferedReader(new FileReader(branchFile));
		line = br.readLine();
		if (line == null){
			System.out.println("店舗ファイルの中身が空です");
			return;
		}
		while ((line = br.readLine()) != null) {
			String[] datas = line.split(",");
			if (!datas[0].matches("^[0-9]{3}$") || datas.length != 2) {
				System.out.println("店舗ファイルのフォーマットが不正です");
				return;
			}
			Map<String, String>map = new HashMap<String, String>();
			map.put("branchCode", datas[0]);//keyがbranchCodeでvalueがdatas[0]
			map.put("branchName", datas[1]);
			branchs.add(map);
		}
	} catch (IOException e) {
		e.printStackTrace();
		return;
	} finally {
		try {
			br.close();
		} catch (Exception e) {
		}
	}
	//商品定義の処理
	File itemFile = new File(dir + ITEM);
	if (!itemFile.exists()) {
		System.out.println("商品ファイルが存在しません");
		return;
	}
	List<Map<String, String>>items = new ArrayList<Map<String, String>>();
	try {
		br = new BufferedReader(new FileReader(itemFile));
		line = br.readLine();
		if (line == null){
			System.out.println("商品ファイルの中身が空です");
			return;
		}
		while ((line = br.readLine()) != null) {
			String[] datas = line.split(",");
			if (!datas[0].matches("^[0-9a-zA-Z]{8}") || datas.length != 2) {
				System.out.println("商品ファイルのフォーマットが不正です");
				return;
			}
			Map<String,String>map = new HashMap<String, String>();
			map.put("itemCode", datas[0]);
			map.put("itemName", datas[1]);
			items.add(map);
		}
	} catch (IOException e) {
		e.printStackTrace();
		return;
	} finally {
		try {
			br.close();
		} catch (Exception e) {
		}
	}

｝
という処理をメインメソッドの中で行っているのですが、
比べると処理はほぼ同じです。このほぼ同じ処理をメソッドにすることは可能でしょうか？
可能ならば、どこをメソッド化できますか？
稚拙な質問内容ではありますが、ご回答お願い致します。

Answer 1

二つを横に並べて、違うとこを全部洗い出す。まずそのぶぶんが外から与えたパラメーターに置き換え可能か検討する。
次に、カラムのバリデーション部分、二つのファイル共２カラムだし、同じ判定だからここは除外するか、考え方をちょっと捻って、カラムは幾つでもどんあ順序でも雛形さえ用意しておけば勝手にバリデーションして値を格納した結果を返す処理に応用できるか、と思い至れば、同一記述部分を抽象化できます。

前回の質問回答で、ファイルサイズ0の判定方法を教わったはずですが、正しく実装出来ていませんね。教わったことは自分なりに吸収してソースをグレードアップしないと、前回教えてくれた人からの意見が聞けなくなりますよ。

Comments

[kurageno]

ご回答頂きありがとうございます。
教えて頂いた考え方を参考に、ソースを書いてみます。
そしてご指摘頂きありがとうございます。
前回の教えて頂いたのと別のやり方でできたのでいいかな・・・？と安易に思ってしまいました。
以後気をつけます。

Answer 2

かなり雑ですが。

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class Sample {
	private static final String BRANCH = "branch";
	private static final String ITEM = "item";

	public static void main(String[] args) {

		String dir = args[0];
		dir = dir + "\\";

		// 店舗定義の処理
		meth( dir, BRANCH, "店舗", "^[0-9]{3}$" );
		
		// 商品定義の処理
		meth( dir, ITEM, "商品", "^[0-9a-zA-Z]{8}" );
	}

	private static void meth(String dir, String type, String type_name, String check_string ) {
		BufferedReader br = null;
		String line = "";

		File file = new File(dir + type);
		if (!file.exists()) {
			System.out.println(type_name + "ファイルが存在しません");
			return;
		}
		List<Map<String, String>> values = new ArrayList<Map<String, String>>();
		try {
			br = new BufferedReader(new FileReader(file));
			line = br.readLine();
			if (line == null) {
				System.out.println(type_name + "ファイルの中身が空です");
				return;
			}
			while ((line = br.readLine()) != null) {
				String[] datas = line.split(",");
				if (!datas[0].matches(check_string) || datas.length != 2) {
					System.out.println(type_name + "ファイルのフォーマットが不正です");
					return;
				}
				Map<String, String> map = new HashMap<String, String>();
				map.put("code", datas[0]); // keyがcodeでvalueがdatas[0]
				map.put("name", datas[1]);
				values.add(map);
			}
		} catch (IOException e) {
			e.printStackTrace();
			return;
		} finally {
			try {
				br.close();
			} catch (Exception e) {
				e.printStackTrace();
				return;
			}
		}

	}
}

手順としては

• 双方の処理をとりあえずメソッド化する
• エラーが出る足りない変数を引数にする（面倒でなければそれぞれ別ファイルにする）
• 双方の関数を比較する（ファイルを比較ツールにかける）
• 相違点で引数化できるものを引数化する

といった感じです。
提示されたコードの内容で、共通化のための処理の整理は七割がた終わっている印象で、
ほぼ機械的に作業するだけでできました。

全体のコードの内容を見る限り、それなりに Java を使いこまれている印象で、
双方の処理も同じ処理は同じ形式になるように整理されていて、
この程度のことができない方とは思えないのですが・・・

自分にはできないと思い込んでいるだけなのでは？

Comments

[kurageno]

ご回答頂きありがとうございます。
手順もとてもわかりやすく有りがたいです。

恥ずかしながら、自分の書いてるソースを理解できていないまま書いているので、
基礎ができていないんです。要勉強ですよね・・・
自信も持てるよう頑張ります。
ありがとうございます。

Answer 3

自分だったらですが、こんな感じですかね・・・コンパイルとか全然してないのですが。。

	//テキストファイルの内容読みだして、各行リストに保持して返す
	private static List<String> readTextFile(String pass){
		File f = new File(pass);
		if(!f.exists()) return null;
		
		List<String> textlines = new ArrayList<String>();
		BufferedReader br = null;
		try{
			br = new BufferedReader(new FileReader(f));
			String line = "";
			while((line = br.readLine()) != null){
				textlines.add(line);
			}
			return textlines;
		}catch (IOException e) { 
			//とりあえず異常終了させちゃってます
			throw new RuntimeException(e);
		}finally{
			br.close();
		}
	}
	
	private static List<Map<String, String>> createShopListMap(List<String> textlines){
		
		List<Map<String, String>> branchs = new ArrayList<Map<String, String>>();
		for(String line : textlines){
			String[] datas = line.split(",");
			if (datas.length != 2 || !datas[0].matches("^[0-9]{3}$")) 
				throw new RuntimeException("店舗ファイルのフォーマットが不正です");
			
            Map<String, String>map = new HashMap<String, String>(); 
            map.put("branchCode", datas[0]);//keyがbranchCodeでvalueがdatas[0] 
            map.put("branchName", datas[1]); 
            branchs.add(map); 
		}
		return branchs;
	}
	
	private static List<Map<String, String>> createItemListMap(List<String> textlines){
		
		List<Map<String, String>> items = new ArrayList<Map<String, String>>();
		for(String line : textlines){
			String[] datas = line.split(",");
			if (datas.length != 2 || !datas[0].matches("^[0-9a-zA-Z]{8}")) 
				throw new RuntimeException("商品ファイルのフォーマットが不正です");
			
            Map<String, String>map = new HashMap<String, String>(); 
            map.put("itemCode", datas[0]);//keyがbranchCodeでvalueがdatas[0] 
            map.put("itemName", datas[1]); 
            items.add(map); 
		}
		return items;
	}
	
	
	
	/**
	 * @param args
	 */
	public static void main(String[] args) throws IOException{

		String dirName = args[0];
		
		String branchFileName = String.format("%s\\%s", dirName, BRANCH);
		List<String> branchTextBody = readTextFile(branchFileName);
		if(branchTextBody.isEmpty())
			throw new RuntimeException("店舗ファイルの中身が空です");
		
		List<Map<String,String>> branches = createShopListMap(branchTextBody);
		
		String itemFileName = String.format("%s\\%s", dirName, ITEM);
		List<String> itemTextBody = readTextFile(itemFileName);
		if(itemTextBody.isEmpty())
			throw new RuntimeException("商品ファイルの中身が空です");
		
		List<Map<String,String>> items = createItemListMap(itemTextBody);
		
	}

メソッド名センスなくて恥ずかしいのですが・・・仕事を分析すると
・ファイルからテキストデータを読みだす
・読みだしたデータから、MAPのLISTを作る
という作業があるから、各々切り出しました。

店舗のデータと、商品のデータは、今は同じでも別々になる可能性もあるので、
このデータを構築する作業を安易に同一の処理にしないほうが良いです。

一方、「テキストファイル内容を一気に読みだす」という処理はコンピュータ寄りなので、
共通のメソッドにするのがふさわしいです。

Comments

[kurageno]

ご回答頂きありがとうございます。
Listを使ってメソッドの作成方法もできるんですね・・・

Answer 4

多分似た処理を書くときに、コピペして違う部分だけ書き換えたと思います。
コピペした部分全体が抜き出せます。
大体の場合、書き換えた部分は引数か戻り値になります。

Comments

[kurageno]

ご回答頂きありがとうございます。
戻り値、引数を使って考えてみます！