Как я могу переписать этот код, чтобы улучшить его ясность?

Question

Не могли бы вы написать этот «чище»? Просто простой вопрос от новичка :)

if(isset($_GET['tid']) && trim($_GET['tid'])!==""){ 
$act = 'tid'; 
$tid = trim($_GET['tid']); 

}elseif(isset($_GET['fid']) && trim($_GET['fid'])!==""){ 
$act = 'fid'; 
$fid = trim($_GET['fid']); 

}elseif(isset($_GET['mid']) && trim($_GET['mid'])!==""){ 
$act = 'mid'; 

}elseif(isset($_GET['act']) && trim($_GET['act'])!==""){ 
$act = trim($_GET['act']); 

}else{ 
$act = ""; 
} 

Answer 1

Я хотел бы сделать это следующим образом:

$tid = isset($_GET['tid']) ? trim($_GET['tid']) : ''; 
$fid = isset($_GET['fid']) ? trim($_GET['fid']) : ''; 
$mid = isset($_GET['mid']) ? trim($_GET['mid']) : ''; 
$act = isset($_GET['act']) ? trim($_GET['act']) : ''; 

if (empty($act)) // act not set, construct the act from the other GET vars 
{ 
    if (!empty($tid)) 
     $act = 'tid'; 
    else if (!empty($fid)) 
     $act = 'fid'; 
    else if (!empty($mid)) 
     $act = 'mid'; 
} 

редактировать: Конечно, вы могли бы сделать это еще короче, но вопрос в том, как это могло быть записано в «улучшить свою ясность». И я понимаю ясность как нечто, что облегчает понимание, что происходит в части кода. И я думаю, что фактическая логика исходного кода становится понятной с моим решением.

Answer 2

Определенно не «чистый» решение, но намного короче:

$act = ''; 
foreach(array('tid', 'fid', 'mid', 'act') as $a) { 
    if(isset($_GET[$a]) && strlen(trim($_GET[$a])) > 0) { 
     $$a = trim($_GET[$act = $a]); 
     break; 
    } 
} 

Answer 3

Я не вижу ничего плохого в вашем коде, кроме отсутствия отступа:

if(isset($_GET['tid']) && trim($_GET['tid'])!==""){ 
    $act = 'tid'; 
    $tid = trim($_GET['tid']); 

}elseif(isset($_GET['fid']) && trim($_GET['fid'])!==""){ 
    $act = 'fid'; 
    $fid = trim($_GET['fid']); 

}elseif(isset($_GET['mid']) && trim($_GET['mid'])!==""){ 
    $act = 'mid'; 

}elseif(isset($_GET['act']) && trim($_GET['act'])!==""){ 
    $act = trim($_GET['act']); 

}else{ 
    $act = ""; 
} 

Хотя, возможно, вы могли бы извлечь выгоду из такая функция

function get_non_empty($field){ 
    return isset($_GET[$field]) && trim($_GET[$field])!='' ? $_GET[$field] : NULL; 
} 

Answer 4

Это один из способов. Тем не менее, я бы, вероятно, сделал что-то по-другому с материалом tid, fid, mid, если бы знал, для чего они предназначены.

list($act,$val) = firstValidGETIn('tid','fid','mid','act'); 
switch($act) { 
    case 'act': $act = $val; break; 
    case null : $act = ""; break; 
    default : $$act = $val; 
} 

function firstValidGETIn() 
{ 
    foreach(func_get_args() as $key) 
    { 
     if(array_key_exists($key,$_GET) && trim($_GET[$key])) 
      return array($key, trim($_GET[$key])); 
    } 
    return array(null,null); 
} 

Answer 5

Это почти идентично логично, что тыкать сделал (+1 за совать за избиение меня к нему), но так как мы говорим о ясность я думал, что я хотел бы показать свое взятие на нем. Мне нравится использовать FALSE вместо пустых строк, когда это означает, что что-то не используется. Это похоже на более явный способ сказать «нет». Кроме того, я редко использую не-скопированную версию if/else, но для действительно коротких операторов присваивания мне легче читать.

$tid = isset($_GET['tid']) ? trim($_GET['tid']) : FALSE; 
$fid = isset($_GET['fid']) ? trim($_GET['fid']) : FALSE; 
$mid = isset($_GET['mid']) ? trim($_GET['mid']) : FALSE; 
$act = isset($_GET['act']) ? trim($_GET['act']) : FALSE; 

if ($act){ // act not set, construct the act from the other GET vars 
    if ($tid)  $act = 'tid'; 
    else if ($fid) $act = 'fid'; 
    else if ($mid) $act = 'mid'; 
}

Answer 6

Осторожно с этими необработанными значениями GET. Вы должны очистить эти значения до их обработки, чтобы убедиться, что вы получаете именно то, что хотите, особенно если это необходимо для вставки значений в базу данных.