2011-09-26 83 views
2

即時通訊方式存在一些問題,目前我的用戶錯誤mensage。我用兩個Goto指令「解決」了問題。請看看代碼:GOTO是一種很好的做法嗎? (在這個PHP的特殊情況?)

<?php require_once("registration/include/membersite_config.php"); ?> 
<!DOCTYPE html> 
<html lang="en"> 
<head><?php include_once("parts/head.php"); ?></head> 
    <body><div id="footerfix"> 
    <?php include_once("parts/header.php"); ?> 
    <div class="container"> 
     <div class="hero-unit"> 
<?php 
if(isset($_GET['i'])){ unlink("users/thumbs/".$_SESSION["user_code"].".jpg"); header('Location: profile.php?i=mycv');} 
if(isset($_FILES['avatar']['tmp_name'])){ 
    $file_ext = end(explode('.',$_FILES['avatar']['name'])); 
    if(in_array($file_ext,array('jpg','jpeg','png','gif'))==false){echo("<h2>Error!</h2><p>Your profile photo have to be a picture file.</p>"); goto nomore;} 
    $src_size=getimagesize($_FILES['avatar']['tmp_name']); 
    if($src_size['mime']=='image/jpeg') {$src_img=imagecreatefromjpeg($_FILES['avatar']['tmp_name']); 
    } elseif($src_size['mime']=='image/png') {$src_img=imagecreatefrompng($_FILES['avatar']['tmp_name']); 
    } elseif($src_size['mime']=='image/gif') {$src_img=imagecreatefromgif($_FILES['avatar']['tmp_name']); 
    } else {echo("<h2>Error!</h2><p>Incorrect file format.</p>"); goto nomore;} 
    $thumb_w = 150; 
    if($src_size[0]<=$thumb_w){$thumb=$src_img; 
    }else{ 
     $new_size[0] = $thumb_w; 
     $new_size[1] = ($src_size[1]/$src_size[0])*$thumb_w; 
    $thumb=imagecreatetruecolor($new_size[0],$new_size[1]); 
    imagecopyresampled($thumb,$src_img,0,0,0,0,$new_size[0],$new_size[1],$src_size[0],$src_size[1]); 
    } 
    imagejpeg($thumb,"users/thumbs/".$_SESSION["user_code"].".jpg"); 
    //header('Location: profile.php?i=mycv'); 
    echo('<h2>Ready!</h2><p>Your profile picture is updated. <a href="profile.php">Go back</a>.</p>'); 
    nomore: echo "</div></div>"; include_once("parts/footer.php"); echo "</div></body></html>"; 
} 
?> 

我永遠無法理解爲什麼GOTO是可能發生的代碼(至少,每一個說)最壞的想,我會想聽聽你對這個意見,如果這真的是最糟糕的想法,那麼我的代碼如何使用它們呢?謝謝!

enter image description here

+2

我會說答案是「不」。 –

+1

請參閱Steve McConnell的Code Complete中的http://www.stevemcconnell.com/ccgoto.htm。當你有時間時閱讀整本書。 –

+10

抱歉是粗魯的,但使用'goto'是你的問題中最少的。最明顯的問題是,代碼首先是完全不可讀的。 –

回答

7

簡短的回答爲什麼GOTO是一個壞主意:可讀性受損。試想一下:

<?php require_once("registration/include/membersite_config.php"); ?> 
<!DOCTYPE html> 
<html lang="en"> 
<head><?php include_once("parts/head.php"); ?></head> 
    <body><div id="footerfix"> 
    <?php include_once("parts/header.php"); ?> 
    <div class="container"> 
     <div class="hero-unit"> 
<?php 
if(isset($_GET['i'])){ unlink("users/thumbs/".$_SESSION["user_code"].".jpg"); header('Location: profile.php?i=mycv');} 
if(isset($_FILES['avatar']['tmp_name'])){ 
    $file_ext = end(explode('.',$_FILES['avatar']['name'])); 
    if(in_array($file_ext,array('jpg','jpeg','png','gif'))==false){ 
     echo("<h2>Error!</h2><p>Your profile photo have to be a picture file.</p>"); 
    } 
    else { 
     $src_size=getimagesize($_FILES['avatar']['tmp_name']); 
     if($src_size['mime']=='image/jpeg') { 
      $src_img=imagecreatefromjpeg($_FILES['avatar']['tmp_name']); 
     } elseif($src_size['mime']=='image/png') { 
      $src_img=imagecreatefrompng($_FILES['avatar']['tmp_name']); 
     } elseif($src_size['mime']=='image/gif') { 
      $src_img=imagecreatefromgif($_FILES['avatar']['tmp_name']); 
     } else { 
      echo("<h2>Error!</h2><p>Incorrect file format.</p>"); 
     } 
     if(!empty($src_img)) { 
      $thumb_w = 150; 
      if($src_size[0]<=$thumb_w){ 
       $thumb=$src_img; 
      }else{ 
       $new_size[0] = $thumb_w; 
       $new_size[1] = ($src_size[1]/$src_size[0])*$thumb_w; 
       $thumb=imagecreatetruecolor($new_size[0],$new_size[1]); 
       imagecopyresampled($thumb,$src_img,0,0,0,0,$new_size[0],$new_size[1],$src_size[0],$src_size[1]); 
      } 
      imagejpeg($thumb,"users/thumbs/".$_SESSION["user_code"].".jpg"); 
      //header('Location: profile.php?i=mycv'); 
      echo('<h2>Ready!</h2><p>Your profile picture is updated. <a href="profile.php">Go back</a>.</p>'); 
     } 
    } 
} 
?> 
    </div></div> 
    <?php include_once("parts/footer.php"); ?> 
</div> 
</body> 
</html> 

無論如何,你應該考慮從邏輯分離模板(谷歌「MVC」),並至少使用功能複雜的操作。

+0

P.S .:代碼中的標題行不起作用,因爲您已經開始輸出。 – fboes

+0

+1 for MVC!.... – ComFreek

0

GOTO在任何地方都不是好的做法。

+3

_GOTO不是一個好的做法where'_ [[需要的引證]'我不反對,但是這裏需要更多的實質... –

+4

這是一個聲明宗教信仰。在某些情況下,使用goto並不是什麼大問題,並且可能是實現控制流程最可讀的方式。還有一些情況(大多數情況下)使用goto是不合適的。 – Hammerite

0

在我看來,continuebreak將是可取的。如果您試圖使用goto,請考慮腳本的流程,可能會改進它。

1

Goto通過創建意大利麪代碼來引入問題!換句話說,您將很難理解代碼塊中代碼的流向!始終可以使用結構化的控制結構並完全消除gotos!正如着名的計算機科學家埃德格瓦迪克斯特拉所言,在他的1968年的文章「反對轉向聲明的案例」(編輯Niklaus Wirth改爲「轉向聲明被認爲是有害的」一文中)中暗示「轉到」被認爲是有害的。以控制結構的維基百科條目http://en.wikipedia.org/wiki/Control_structures爲出發點。

該概念適用於所有編程語言!

+0

確實總是可以使用控制結構來消除gotos,但是這樣做總是會改善代碼的可讀性嗎? – Hammerite

+1

@Hammerite我認爲答案几乎是普遍的,因爲不同於所有其他流控制原語,goto不具有局部性 - 它可以跳轉到任何地方,與代碼結構無關。我能想到的唯一例子是使用計算得到的gotos的字節碼解釋器。 –

+0

的確,「goto沒有表現出地方性」,但它可能以一種有助於「代碼結構」的方式使用;例如,允許執行跳轉到位於過程主邏輯之後的錯誤處理塊。在這種情況下,代碼可讀性不會通過替換goto而顯着提高嗎? – Hammerite

5

您使用goto在這裏似乎合理的,我的,因爲你基本上採用以下模式:

if (error_condition_1) { 
    goto handle_errors; 
} else { 
    // do stuff 
} 
if (error_condition_2) { 
    goto handle_errors; 
} else { 
    // do stuff 
} 
// ... 
if (last_error_condition) { 
    goto handle_errors; 
} else { 
    // do stuff 
} 
// do stuff 
handle_errors: 
    // handling errors 

還有其他的方法可以做到這一點不涉及跳轉;例如,您可以在適當的位置使用do ... while (false)循環和break語句。但是,這實際上掩蓋了您使用轉到的事實。

goto在計算機程序員中具有賤民的地位,這些日子在很大程度上是不公平的;在某些情況下,使用goto是一個非常合理的選擇(不是大多數情況下,我急於添加),但總的來說,使用goto很容易讓你感到沮喪,不管它是否是有效的設計選擇。

正如丹尼爾布羅克曼所說,您的主要問題在於您的代碼很難閱讀。它似乎被擠在儘可能少的線上,並且非常密集。看看其他人如何在他們的網站和其他地方放置他們的代碼,看看你是否同意使用其他約定的代碼更具可讀性。

在PHP中,您需要記住goto只能從5.3版本獲得,因此如果您的代碼需要可移植,您可能需要考慮其他選項。

+1

Daniel Brockman似乎刪除了他的答案,但我不知道爲什麼。 - 哦,這是一個評論,而不是一個答案。忽略此評論 – Hammerite

+0

還應該注意,一旦完成了可讀和記錄的版本,您可以將其打包以便稍後分發以節省空間等。無需將盡可能多的代碼塞入單行中誰編程。 – ShadowScripter

+0

您描述的模式使用異常處理會更好。 –

2

你的代碼亂七八糟。實現一個GOTO語句會讓它成爲一場噩夢。 像XKCD表演一樣,可怕的事情會發生。我記得當我開始學習C++時,術語「意大利麪代碼」通常指代goto的用法。它會一直搞亂你的執行流程。更不用說它毀了可讀性。

在任何情況下,我已經爲您調整了程序,現在它應該可以工作。

腳本


<?php require_once("registration/include/membersite_config.php"); 
    //redirect user if i is defined 
    if(isset($_GET['i'])){ unlink("users/thumbs/".$_SESSION["user_code"].".jpg"); header('Location: profile.php?i=mycv');} 
?> 
<!DOCTYPE html> 
<html lang="en"> 
<head><?php include_once("parts/head.php"); ?></head> 
    <body> 
    <div id="footerfix"> 
     <?php include_once("parts/header.php"); ?> 
     <div class="container"> 
      <div class="hero-unit"> 
<?php 

check_file(); 

function check_file(){ 
    //check for file 
    if(isset($_FILES['avatar']['tmp_name'])){ 
     //get the file extension 
     $file_ext = end(explode('.',$_FILES['avatar']['name'])); 

     //validate extension 
     if(in_array($file_ext,array('jpg','jpeg','png','gif')) ==false){ 
      echo "<h2>Error!</h2><p>Your profile photo have to be a picture file.</p>"; 
      return -1; 
     } 

     //get the image dimensions 
     $src_size = getimagesize($_FILES['avatar']['tmp_name']); 

     switch($src_size['mime']){ 
      case 'image/jpeg': 
       $src_img=imagecreatefromjpeg($_FILES['avatar']['tmp_name']); break; 
      case 'image/png': 
       $src_img=imagecreatefrompng($_FILES['avatar']['tmp_name']); break; 
      case 'image/gif': 
       $src_img=imagecreatefromgif($_FILES['avatar']['tmp_name']); break; 
      default: 
       echo("<h2>Error!</h2><p>Incorrect file format.</p>"); 
       return -1; 
     } 

     $thumb_w = 150; 

     //if image is within allowed dimensions, set thumbnail as image 
     if($src_size[0]<=$thumb_w){ 
      $thumb = $src_img; 
     }else{ 
      $new_size[0] = $thumb_w; 
      $new_size[1] = ($src_size[1]/$src_size[0])*$thumb_w; 
      $thumb= imagecreatetruecolor($new_size[0],$new_size[1]); 
      imagecopyresampled($thumb,$src_img,0,0,0,0,$new_size[0],$new_size[1],$src_size[0],$src_size[1]); 
     } 

     //create the updated image 
     imagejpeg($thumb,"users/thumbs/".$_SESSION["user_code"].".jpg"); 

     echo '<h2>Ready!</h2><p>Your profile picture is updated. <a href="profile.php">Go back</a>.</p>'; 
    } 
} 
?> 
       </div> 
      </div> 
      <?php include_once("parts/footer.php"); ?> 
     </div> 
    </body> 
</html> 

正如你所看到的,我封裝在一個函數中的全過程。這樣,只要遇到錯誤,您就可以執行return語句。使其成爲一項功能還可以實現快速重用。您可以添加一些參數以使其更具動態性。

相關問題