もっと簡潔にできると思うのですが…。

Question

function PCorPhone($ua){
    if(mb_strpos($ua,'Android')===false){
        $css = 'designPC.css';
    }
    if(mb_strpos($ua,'iPhone')===false){
        $css = 'designPC.css';
    }
    if(mb_strpos($ua,'iPad')===false){
        $css = 'designPC.css';
    }
    if(mb_strpos($ua,'Window')===false){
        $css = 'designPhone.css';
    }
    return $css;
}

PCとスマホでアクセスしたのを区別するプログラムを書いたのですが、もっと簡潔な書き方がありませんか？
アドバイスいただけると助かります。よろしくお願いします。

Comments

[退会済みユーザー]

Windows Phone, BlackBerry を忘れないで上げて

Answer 1

PCとスマホの切り分け条件は、ユーザーエージェント（以下 UA）に、
・Android か iPhone か iPad の文字列があれば、スマホ用
・Windows があれば PC 用
でしょうか？

提示されたソースの場合、Android も iPhone も iPad も Windows もない（例えば Mac からだったり Linux 機だったり）場合、$css が未定義のままですから、意図しない表示になるはずです。

上の切り分け条件をちょっと変えて
・Android か iPhone か iPad の文字列があれば、スマホ用
・そうでなければ PC 用
であれば、こういう感じになります。

function PCorPhone($ua) {
  $css = 'designPC.css';
  if (preg_match(/(Android|iPhone|iPad)/, $ua) === 1) {
    $css = 'designPhone.css';
  }
  return $css;
}

Comments

[chabiribon]

なるほど！preg_matchを使えば簡潔ですね！ありがとうございます。早速やってみます。

[chabiribon]

こちらの回答を参考に作成したところ無事にできました！また、いろいろな意見を聞けてとても参考になりました。皆さんありがとうございました！

Answer 2

function PCorPhone($ua){
    if(mb_strpos($ua,'Window')===false){
        $css = 'designPhone.css';
    } else {
        $css = 'designPC.css';
    }
    return $css;
}

Comments

[chabiribon]

あ！そうか！なるほど。確かにこれなら簡潔ですね！

[yambejp]

Window以外がPhoneはさすがに乱暴すぎでは？

[ttyp03]

そうかもしれませんが、提示されたコードを単純に簡潔にしただけですので。

Answer 3

まずもって、このコードでは狙ったとおりに動きません。たとえば、macOSのSafariは以下のようなユーザーエージェントを返します。

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36

最後のWindow条件に引っかかって、パソコンなのに'designPhone.css'を返してしまいます。

こういったものは、自分で書くより既存のライブラリを使ったほうが楽で確実だと思います。

Comments

[chabiribon]

このままではmacOSに対応できていないのですね？既存のライブラリとはどのようなものでしょうか？

Answer 4

$cssのデフォ値が設定されていないですね
またifが排他的になっていないので常に後者で上書きされています。
基本をdesignPC.cssとして、特定の時designPhone.cssにするとよいのでは？

function PCorPhone($ua){
  $css = preg_match('/Android|iPhone|iPad/',$ua)?'designPhone.css':'designPC.css';
  return $css;
}
```;

Comments

[chabiribon]

なるほど、参考になります。ありがとうございます。

Answer 5

tacsheavenさんやyambejpさんのようなロジック変更を行った方が良いかと思いますが、
質問文のソースコードを極限までシンプルにするとこうなります。

// この書き方は規約でアウト扱いの可能性もあり
function PCorPhone($ua){
    if (mb_strpos($ua, 'Window') === false) return 'designPhone.css';
    return 'designPC.css';
}

// その場合はこっち
function PCorPhone($ua){
    if (mb_strpos($ua, 'Window') === false) {
        return 'designPhone.css';
    }
    return 'designPC.css';
}

所謂ガード説という対応です。
さっさとreturn返して逃げれば一時変数も使わずに済んで万々歳ですね。

---

おまけ

PCorPhoneという関数を見て、他の人はどういう値を返す関数と想像するでしょうか？
まぁ大多数は文字列の"PC"か"Phone"なんだろうと想像するでしょうね。
ところが、実行すると返ってくるのが"designPC.css"なわけですよ。

なので下のようなアプローチが良いでしょうね。

function PCorPhone($ua){
    if (mb_strpos($ua, 'Window') === false) {
        return 'Phone';
    }
    return 'PC';
}

$device = PCorPhone($ua);
$css = "design{$device}.css";

function isPC($ua){
    if (mb_strpos($ua, 'Window') === false) {
        return false;
    }
    return true;
}

$device = isPC($ua) ? 'PC' : 'Phone';
$css = "design{$device}.css";

Answer 6

解決しました。これで今のところ上手くいっています。ありがとうございました。

//パソコンかスマホかを判断する関数
function PCorPhone($ua){

    $pattern = "/(Android|iPhone|iPad)/";
    $css = 'designPC.css';

    if(preg_match($pattern, $ua) === 1){
        $css = 'designPhone.css';
    }
    return $css;

}

Comments

[yambejp]

一応念のため今回のパターンにカッコは要らないです

[chabiribon]

そうなんですね。まだまだ理解不足で…。ありがとうございます。