2013/02/09 このエントリーをはてなブックマークに追加 はてなブックマーク - 現場であった怖い話(I experienced scary stroy about programing)

現場であった怖い話(I experienced scary stroy about programing)














今の現場での業務は機能の拡張案件なんですが、
某有名な団体の某APIを拡張するっていうものです。


あんまり具体的な話すると守秘義務がどうのこうのなので、
ちょっとアバウトにクラスを再現してみようと思います。
ユーザー認証のAPIを作っているという体で行きます。


親クラスはこんな感じです。



package com.yank.yy.refactor;

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.util.HashMap;
import java.util.Map;

/**
 * スーパークラスです
 * 
 * @author yy_yank
 * 
 */
public class SuperClassSample {

    /**
     * シングルトンインスタンス
     */
    private static SuperClassSample instance = new SuperClassSample();

    /** ユーザー情報マップ */
    private static Map<String, String> userMap = new HashMap<String, String>();

    /**
     * シングルトンインスタンスを返します.
     */
    public static SuperClassSample getInstance() {
        return instance;
    }

    /** コンストラクタ */
    protected SuperClassSample() {
    }
    

    /**
     * ユーザー認証
     * 
     * @param user
     * @return
     */
    public synchronized boolean isUser(String user) {
        return this.isUser(user, false);
    }

    /**
     * ユーザー認証
     * 
     * @param user
     * @param reset
     * @return
     */
    public synchronized boolean isUser(String user, boolean reset) {

        if (user == null) {
            return false;
        }

        if (reset) {
            resetUserPass(user);
        }

        return userMap.containsKey(user);
    }

    /**
     * ユーザーパス削除
     * 
     * @param user
     */
    public synchronized void resetUserPass(String user) {

        if (userMap == null) {
            return;
        }
        userMap.put(user, null);
    }

    /**
     * ユーザーパス保存
     * 
     * @param user
     */
    public synchronized void saveUserPass(String user) {

        String pass = generateUserString(user);
        if (pass != null) {
            userMap.put(user, pass);
        }

    }

    /**
     * ユーザーパス生成
     * 
     * @param user
     * @return
     */
    public String generateUserString(String user) {

        try {
            byte id[] = user.getBytes();
            byte now[] = new Long(System.currentTimeMillis()).toString().getBytes();
            MessageDigest md = MessageDigest.getInstance("MD5");
            md.update(id);
            md.update(now);
            return this.toHex(md.digest());

        } catch (IllegalStateException e) {
            return null;
        } catch (NoSuchAlgorithmException e) {
            return null;
        }

    }

    /**
     * byte配列をStringに変換
     * 
     * @param buffer
     * @return
     */
    public String toHex(byte buffer[]) {
        StringBuffer sb = new StringBuffer();
        String s = null;
        for (int i = 0; i < buffer.length; i++) {
            s = Integer.toHexString((int) buffer[i] & 0xff);
            if (s.length() < 2) {
                sb.append('0');
            }
            sb.append(s);
        }
        return sb.toString();
    }

}





これを拡張するにあたって、設計者の見本コーディングがこれ、



package com.yank.yy.refactor;

import java.util.HashMap;
import java.util.Map;

/**
 * スーパークラスの機能を引き継ぐクラスです(ビフォー)
 * 
 * @author yy_yank
 * 
 */
public class ModifyClassBefore {

    /** ユーザー格納キーパラメータ名 */
    public static final String USER_KEY = "com.yank.yy.USER_KEY";

    /** ユーザーパラメータ */
    public static final String USER_VALUE = "com.yank.yy.USER_VALUE";

    /** ユーザー情報マップ */
    private static Map<String, String> userMap = new HashMap<String,String>();

    /** シングルトンインスタンス */
    private static ModifyClassBefore instance = new ModifyClassBefore();

    /**
     * プライベートコンストラクタ(不可視)
     */
    private ModifyClassBefore() {
    }

    /**
     * シングルトンインスタンス取得
     * 
     * @return シングルトンインスタンス
     */
    public static ModifyClassBefore getInstance() {
        return instance;
    }

    /**
     * ユーザーパス生成
     * 
     * @param user
     * @return
     */
    public String generateUserString(String user) {
        return SuperClassSample.getInstance().generateUserString(user);
    }

    /**
     * ユーザーパス削除
     * 
     * @param user
     *            String
     */
    public synchronized void resetUserPass(String user) {
        if (userMap != null) {
            // USER格納キー取得
            String user_key = userMap.get(USER_KEY);
            userMap.put(userMap.get(user_key), null);
        }
    }

    /**
     * ユーザーパス生成&保存<br>
     * 再生成フラグtrueの場合、新規にパスを生成し保存する<br>
     * 再生成フラグfalseの場合、既存パスを利用する。
     * 
     * @param user
     * @param reset
     */
    public synchronized void saveUserPass(String user, boolean reset) {
        // Userジェネレータ取得
        SuperClassSample superClass = SuperClassSample.getInstance();
        String user_key;
        String user_value;
        if (reset) {
            // ユーザーパス削除
            resetUserPass(user);
            // User格納キー生成
            user_key = superClass.generateUserString(user);
            // User生成
            user_value = superClass.generateUserString(user);
        } else {
            // パス格納キーを取得
            user_key = userMap.get(USER_KEY);
            if (user_key == null) {
                // ユーザーパラメータが存在しない場合、パス格納キーを生成
                user_key = superClass.generateUserString(user);
            }
            // 保存ユーザーパス取得
            user_value = userMap.get(user_key);
            if (user_value == null) {
                // 保存パスが存在しない場合、生成
                user_value = superClass.generateUserString(user);
            }
        }
        // ユーザー格納キーを設定
        userMap.put(USER_KEY, user_key);
        // MAPからユーザーkeyを取得しパス格納
        userMap.put(userMap.get(user_key), user_value);
    }

    /**
     * ユーザーパス生成&保存<br>
     * リセットフラグtrue(パス再生成)でユーザーパスを生成&保存する。
     * 
     * @param user
     */

    public synchronized void saveUserPass(String user) {
        saveUserPass(user, true);
    }

    /**
     * ユーザー判定<br>
     * 
     * @param user
     * @param reset
     * @return
     */
    public synchronized boolean isUser(String user, boolean reset) {
        if (userMap == null) {
            return false;
        }
        // パス格納キー取得
        String user_key = userMap.get(USER_KEY);
        if (user_key == null) {
            return false;
        }
        String user_value = userMap.get(USER_VALUE);
        if (user_value == null) {
            return false;
        }
        // パス取得
        String pass = userMap.get(user_key);
        if (pass == null) {
            return false;
        }
        // リセットフラグが指定されている場合、削除
        if (reset) {
            resetUserPass(user);
        }
        // パスの比較結果を返す
        return user.equals(user_value);

    }

    /**
     * ユーザー判定<br>
     * チェック時にパスを削除しない。
     * 
     * @param user
     * @return
     */
    public synchronized boolean isUser(String user) {
        return isUser(user, false);
    }

}






おいおい。クラスのシグネチャがほとんど同じじゃないですか。 なぜ継承しない・・・!!

僕ならこういう感じが良かったです。





package com.yank.yy.refactor;

import java.util.HashMap;
import java.util.Map;

/**
 * スーパークラスの機能を引き継ぐクラスです(アフター)
 * 
 * @author yy_yank
 * 
 */
public class ModifyClassAfter extends SuperClassSample {

    /** ユーザー格納キーパラメータ名 */
    public static final String USER_KEY = "com.yank.yy.USER_KEY";

    /** ユーザーパラメータ */
    public static final String USER_VALUE = "com.yank.yy.USER_VALUE";

    /** ユーザー情報マップ */
    private static Map<String, String> userMap = new HashMap<String, String>();

    /**
     * プライベートコンストラクタ(不可視)
     */
    private ModifyClassAfter() {
    }

    /**
     * ユーザーパス削除
     * 
     * @param user
     *            String
     */
    @Override
    public synchronized void resetUserPass(String user) {
        if (userMap != null) {
            // USER格納キー取得
            String user_key = userMap.get(USER_KEY);
            userMap.put(userMap.get(user_key), null);
        }
    }

    /**
     * ユーザーパス生成&保存<br>
     * 再生成フラグtrueの場合、新規にパスを生成し保存する<br>
     * 再生成フラグfalseの場合、既存パスを利用する。
     * 
     * @param user
     * @param reset
     */
    public synchronized void saveUserPass(String user, boolean reset) {
        // Userジェネレータ取得
        SuperClassSample superClass = SuperClassSample.getInstance();
        String user_key;
        String user_value;
        if (reset) {
            // ユーザーパス削除
            resetUserPass(user);
            // User格納キー生成
            user_key = superClass.generateUserString(user);
            // User生成
            user_value = superClass.generateUserString(user);
        } else {
            // パス格納キーを取得
            user_key = userMap.get(USER_KEY);
            if (user_key == null) {
                // 存在しない場合、パス格納キーを生成
                user_key = superClass.generateUserString(user);
            }
            // 保存ユーザーパス取得
            user_value = userMap.get(user_key);
            if (user_value == null) {
                // 保存パスが存在しない場合、生成
                user_value = superClass.generateUserString(user);
            }
        }
        // ユーザー格納キーを設定
        userMap.put(USER_KEY, user_key);
        // MAPからユーザーkeyを取得しパス格納
        userMap.put(userMap.get(user_key), user_value);
    }

    /**
     * ユーザーパス生成&保存<br>
     * リセットフラグtrue(パス再生成)でユーザーパスを生成&保存する。
     * 
     * @param user
     */
    @Override
    public synchronized void saveUserPass(String user) {
        saveUserPass(user, true);
    }

    /**
     * ユーザー判定<br>
     * 
     * @param user
     * @param reset
     * @return
     */
    @Override
    public synchronized boolean isUser(String user, boolean reset) {
        if (userMap == null) {
            return false;
        }
        // パス格納キー取得
        String user_key = userMap.get(USER_KEY);
        if (user_key == null) {
            return false;
        }
        String user_value = userMap.get(USER_VALUE);
        if (user_value == null) {
            return false;
        }
        // パス取得
        String pass = userMap.get(user_key);
        if (pass == null) {
            return false;
        }
        // リセットフラグが指定されている場合、削除
        if (reset) {
            resetUserPass(user);
        }
        // パスの比較結果を返す
        return user.equals(user_value);

    }
}



総ステップ数も30ぐらい減り、スッキリ。
(それでもまぁ糞コーディングですが)

しかし、設計書レビューはとおり、「継承」ではなく「依存」したクラス設計に。


いやだなー。。。早く設計とかできるようになりたいなと思ったのでした。












0 件のコメント:

コメントを投稿