2010-12-05 24 views
0

PHP:より洗練されたコードを作成する

私はこれを持っています:

<?php 
if($_GET['wE'] && is_numeric($_GET['wE'])){ 
$we = mysql_real_escape_string($_GET['wE']); 
$query_find_we = "SELECT id FROM users_wall WHERE id = '$we' AND uID = '$showU[id]'"; 
$query_find_we = mysql_query($query_find_we)or die(mysql_error()); 
$grab_wall_comment = (mysql_num_rows($query_find_we) == 1) ? "window.location.hash = '#comment$we';" : "alert('Vägginlägg kunde ej hittas.');"; 
?> 
<script> 
$(function() { 
<?php echo $grab_wall_comment; ?> 
}); 
</script> 
<?php 
}elseif($_GET['sE'] && is_numeric($_GET['sE'])){ 
$se = mysql_real_escape_string($_GET['sE']); 
$query_find_se = "SELECT id FROM users_statuslog WHERE id = '$se' AND uID = '$showU[id]'"; 
$query_find_se = mysql_query($query_find_se)or die(mysql_error()); 
$grab_status_comment = (mysql_num_rows($query_find_se) == 1) ? "window.location.hash = '#comment$se';" : "alert('Status kunde ej hittas.');"; 
?> 
<script> 
$(function() { 
<?php echo $grab_status_comment; ?> 
}); 
</script> 
<?php 
    } 
?> 

は$ _GET ['we']か$ _GET ['se']が存在するかどうかを調べます。 <script>を実行します。

これを短くする方法はありますか?私はブール値を作成することで自分自身をしようとしましたが、もっと短くすることもできますか?私は将来的にはよりクリーンなコーディングをしたいと思っています。

+0

関数にラップしてください。 – Radu

+0

関数にラップすることで、どういう意味になっているのか解答してください。 – Karem

+1

まず、適切なコードインデントを覚えなければなりません。あなたの現在のコードは完全に読めません。 –

答えて

1

あなたはそれを自分で見ていないのですか?

明らかに2つの同一のコード部分があります。
このコードの可変部分をPHP変数にするだけです。それで全部です。
もちろん、いくつかのdb API関数を使用する必要があります。

<? 
if (!empty($_GET['wE'])) { 
    $id = $_GET['wE'] 
    $table = "users_wall"; 
    $alert = "Vagginlagg kunde ej hittas."; 
} elseif (!empty($_GET['sE'])) { 
    $id = $_GET['sE']; 
    $table = "users_statuslog"; 
    $alert = "Status kunde ej hittas."; 
} 
$query = "SELECT count(id) FROM `$table` HERE id = %d AND uID = %d"; 
$count = db::getOne($query,$id,$showU['id']); 

//Separate your main PHP logic from presentation as much as possible. 
//leave only necessary operators. 
?> 
<script> 
$(function() { 
<? if($count): ?>window.location.hash = '#comment<?=$id?>'; 
<? else: ?>alert('<?=$alert?>'); 
<? endif ?> 
}); 
</script> 

にgetone()関数はgetarrに似ている()私はthis answerで述べたが、配列の代わりにスカラー値を返します。

+0

"主なPHPロジックを可能な限りプレゼンテーションから分離してください。" - それを別のファイルに入れるのはどうですか? –

+0

@ケンドール問題ありません。コメントの下のすべてを別のファイルに切り取り、代わりにインクルードコールを配置するだけです。 –

+0

答えのあなたのコメントについては、私はメモ帳を使用しています+ +これはなぜ私のコードは読めないですか?それとも何を意味しましたか? – Karem

0
<?php 
    $param = (isset($_GET['wE']) && is_numeric($_GET['wE']))?"wE":((isset($_GET['sE']) && is_numeric($_GET['sE']))?"sE":false); 
    if($param){ 
     echo showScript($param); 
    } else { 
     //TODO: do something if there's no 'wE' nor 'sE' maybe? 
    } 

    function showScript($param){ 
     if($param == "wE"){ 
      $table = "users_wall"; 
      $alert = "Vägginlägg kunde ej hittas."; 
     } else { 
      $table = "users_statuslog"; 
      $alert = "Status kunde ej hittas."; 
     } 
     $e = mysql_real_escape_string($_GET[$param]); 
     $query_find = "SELECT id FROM ".$table." WHERE id = '".$e."' AND uID = '$showU[id]'"; 
     $query_find = mysql_query($query_find)or die(mysql_error()); 
     $grab_status_comment = (mysql_num_rows($query_find) == 1) ? "window.location.hash = '#comment$e';" : "alert('$alert');"; 
     return '<script>$(function() {'.$grab_wall_comment.'});</script>'; 
    } 
    ?> 

私はそれをテストしていません...しかし、それは主なアイデアです:P

幸運、これは

を役に立てば幸い
関連する問題