最近見た残念なコード。なお、プロダクションで使われているコードなので、意味内容を変えつつ誇張して書いているため、実際はここまでひどくはない。
class App {
Map<String, String[]> map;
public void add(String protocol, String base, String[] paths) {
for (String path : paths) {
String[] items = new String[3];
if (!map.containsKey(path)) {
items[Constants.APP_URL_PROTOCOL] = protocol;
String sep = paths.startsWith("/")? "" : "/";
items[Constants.APP_URL_URL] = protocol.concat("://").concat(base).concat(sep).concat(path);
items[Constants.APP_URL_PATH] = path;
map.put(path, items);
}
}
}
public boolean storeUrls() {
for(String key : map.keySet()) {
String[] infos = map.get(key);
}
}
}
他のところで取り出した URL
を保存する処理なのだが、 URL
だけでなく、プロトコルとかパスなども保存しているらしい。それは特段問題ではないのだが、問題なのはこれを URL
を使えば意味も明確に処理できるのに対して、 String
と配列という抽象的な要素をやりくりしてアプリを構築しているところである。
class App {
Set<URL> urls;
public void add(String protocol, String base, String[] paths) {
for (String path : paths) {
String sep = paths.startsWith("/")? "" : "/";
URL url = new (protocol, base, sep.concat(path));
urls.add(url);
}
}
public boolean storeUrls() {
for(URL url : urls) {
}
}
}
URL
を使うことによって、何を保持しているか明確になった。もちろん、一部実装が変わっているところがある。それは、パスで一意性を保証していたのに対して、 URL
で一意性を保証するように変わってしまったとことであるが、問題であるようならその場合にはパスによって一意性を保証するような専用の型を作れば良い。
まとめると
String
や配列のような抽象的な型をロジック中でやりくりしてデータを表現しない
- 何でも自力でやらない。標準APIやライブラリーが提供しているものを使う。
- API で提供していない、ライブラリーがないなら、専用の型を定義する。抽象的な型で頑張らない。