2011-09-26 28 views
2

私のユーザーには現在のエラーの人数にはいくつかの問題があります。 2つの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>"; 
} 
?> 

は、私は理解していなかった、私はこのことについて、あなたの意見を聞きたいだろう、もしそれが本当に最悪のものだと思うのですが、どうすれば私のコードを使い続けることができますか?ありがとう!

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 .:既に出力を開始したため、コード内のヘッダーラインは機能しません。 – fboes

+0

+1 for MVC!.... – ComFreek

0

GOTOはどこでも良い習慣ではありません。

+3

_GOTOは良い練習ではないどこでも ''私は同意しないが、ここにもう少し物質が必要である... –

+4

これはステートメントです宗教的信念のgotoを使用することは大したことではなく、制御フローを実装する最も読みやすい方法かもしれないという状況があります。また、gotoの使用が不適切な状況(大多数の状況)もあります。 – Hammerite

0

私の意見では、continueまたはbreakが好ましいでしょう。あなたがgotoを使いたくなったら、あなたのスクリプトの流れを考えると、それは改善される可能性が高いです。

1

スパゲッティコードを作成することで、後藤問題が発生します。言い換えれば、コードブロック内のコードの流れを理解するのは難しいでしょう。構造化されたコントロール構造を使用して、完全に無駄をなくすことは常に可能です!有名なコンピュータ科学者のEdsger W. Dijkstraは、1968年の記事「GO TO Statementに対するケース」(Niklaus Wirthの編集者が「有害なものとみなされる」に改訂された記事)で有害であると考えていることを暗示している。制御構造に関するWikipediaエントリーhttp://en.wikipedia.org/wiki/Control_structuresを出発点として見てください。

この概念は、すべてのプログラミング言語に適用されます。

+0

制御構造を使用してgotosを排除することは常に可能ですが、そうすることでコードの可読性が向上するのはいつものことです。 – Hammerite

+1

@Hammerite私は、他のすべてのフロー制御プリミティブとは異なり、gotoは局所性を示さないため、コードの構造とは無関係にどこにでもジャンプできるため、答えはほぼ普遍的です。私が考えることができる唯一の例は、計算されたgotosを使用するバイトコードインタープリタです。 –

+0

"gotoは局所性を示さない"ことは確かですが、それでも "コードの構造"に貢献する方法で使用することができます。プロシージャのメインロジックの後に位置するエラー処理ブロックに実行をジャンプさせることができます。 gotoを置き換えることによってコードの可読性が大幅に改善されないようなケースでは真実ではありませんか? – Hammerite

5

あなたは基本的に以下のパターンを使用しているので、後藤のご利用には、ここで、私には合理的なようだ:

後藤含まないこれを行うには、他の方法があります
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 

。たとえば、適切な場所にbreakステートメントでループdo ... while (false)を使用することができます。しかし、これは実際にあなたがgotoを使っているという事実をあいまいにしています。

gotoは、コンピュータプログラマーの間では、最近のところ不公平です。 gotoを使用するのは完全に合理的な選択です(追加するのは大部分の状況ではありません)が、一般的にgotoを使用することは、有効なデザインの選択肢であるかどうかにかかわらず、

ここでの主な問題は、Daniel Brockman氏によると、あなたのコードは読みにくいということです。それはあなたが管理できるほどの行に詰め込まれているようで、非常に密集しています。どのように他の人が自分のコードをこのサイトや他の場所に置いているかを調べて、他の規約を使ったコードが読みやすくなるかどうかを確認してください。

PHPでは、gotoはバージョン5.3以降でしか使用できないことに注意する必要があります。したがって、コードを移植する必要がある場合は、別のオプションを検討してください。

+1

Daniel Brockmanは彼の答えを削除したようですが、理由はわかりません。 - ああ、答えではなくコメントだった。このコメントを無視する – Hammerite

+0

読み取り可能で文書化されたバージョンが完成したら、後で配布用にパッケージ化してスペースなどを節約することができます。できるだけ多くのコードを1行に入れる必要はありません誰がそれをプログラムするか。 – ShadowScripter

+0

あなたが記述するパターンは、例外を使って処理するほうがはるかに優れています。 –

2

あなたのコードは混乱しています。 GOTOステートメントを実装すると、それは悪夢になります。 XKCDのショーと同じように、ひどいことが起こります。私がC++を学び始めた時、 "Spaghetti code"という言葉はしばしば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ステートメントを実行できます。それを機能させることで、迅速な再利用が可能になります。より動的にするためにいくつかのパラメータを追加することができます。

関連する問題