リファクタリング案件中、すごくグチャグチャなコードに出会ったので
今の自分の知りうる限りの丁寧さで書いたらどうなるのか気になったので書いてみました。
コードレビューしていただけると嬉しいです。
メインの処理
$ua = $_SERVER['HTTP_USER_AGENT'];
$has = hasStrMaker($ua);
switch (true) {
case isMobile($has):
header("Location: m/");
break;
case isTablet($has):
header("Location: t/");
break;
case isKtai($has):
header("Location: k/");
break;
default:
header("Location: pc/");
break;
}
ライブラリ
// PHP 5.3 以下は"calllable"を外す。
function isMobile(callable $has){
switch (true) {
// Android Phone は "Android" と "Mobile"
case $has('Android') && $has('Mobile'):
case $has('iPhone'):
case $has('Windows Phone'):
return true;
}
return false;
}
function isTablet(callable $has){
switch (true) {
case $has('Android'):
case $has('iPad'):
return true;
}
return false;
}
function isKtai(callable $has){
switch (true) {
case $has('DoCoMo'):
case $has('KDDI'):
case $has('Vodafone'):
return true;
}
return false;
}
function hasStrMaker($s){
return function($n) use(&$s){
if(strpos($s,$n) === false){
return false;
}
return true;
};
}
テスト
<?php
//Tests:
$mobiles = [
// Android Phone
"Mozilla/5.0 (Linux; U; Android 2.3.5; ja-jp; ISW11K Build/145.0.0002) AppleWebKit/533.1 (KHTML, like Gecko) Version/4.0 Mobile Safari/533.1",
// iPhone
"Mozilla/5.0 (iPhone; CPU iPhone OS 6_0_1 like Mac OS X) AppleWebKit/536.26 (KHTML, like Gecko) Mobile/10A523",
// Windows Phone
"Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; KDDI-TS01; Windows Phone 6.5.3.5)"
];
$tablets = [
// iPad
"Mozilla/5.0 (iPad; CPU OS 6_0 like Mac OS X) AppleWebKit/536.26 (KHTML, like Gecko) Version/6.0 Mobile/10A403 Safari/8536.25",
// Android Tablet
"Mozilla/5.0 (Linux; Android 4.1.1; Nexus 7 Build/JRO03S) AppleWebKit/535.19 (KHTML, like Gecko) Chrome/18.0.1025.166 Safari/535.19"
];
$Ktais = [
// DoCoMo
"D501i DoCoMo/1.0/D501i",
// Au
"KDDI-HI31 UP.Browser/6.2.0.5 (GUI) MMP/2.0",
// Vodafone
"Vodafone/1.0/V702NK/NKJ001[/xxx] Series60/2.6 Nokia6630/x.x.x Profile/MIDP-2.0 Configuration/CLDC-1.1"
];
function uaFuncSet_Test($testFunc, $array){
foreach ($array as $ua) {
$has = hasStrMaker($ua);
if($testFunc($has) == true){
echo '.';
}else{strpos($cstr,$t);
echo "fail: $ua";
}
echo "\n";
}
}
$testFunc = isMobile;
uaFuncSet_Test($testFunc, $mobiles);
//uaFuncSet_Test($testFunc, $tablets); // 失敗テスト
$testFunc = isTablet;
uaFuncSet_Test($testFunc, $tablets);
$testFunc = isKtai;
uaFuncSet_Test($testFunc, $Ktais);
isMobile系はチェック関数をシンプルに各中で作るか、クロージャを渡すしてDRYに書くか、迷ったのですが後々の変更やテストのしやすさ(DI的な?)を考えたらDRYを取りました。
読みやすさを少し犠牲にした部分の様な気もしますが大丈夫な範囲でしょうか?