PHP acemi - kod örneği emektar eleştiri arıyor (kod btw çalıştığını, bu bir başlangıç!)

4 Cevap php

SO gezen araya kadarıyla, ben sizin gibi iyi insanların kötü kabusunum: resmen herhangi bir programlama öğrendim hiç birisi, onun hobi web sitesi geliştirmek istiyorum karar, ve kapalı bir fast-track PHP kitabı aldım raf ve ona gitti.

Tarama SO, bana şeylerin bir çok şey öğretti gelmiştir olayları şunlardır:

  • PHP (Ben hangi) yeni başlayanlar için korkunç
  • PHP korkunç bir ilk dil (Bu benim ilk)
  • çok ve PHP kod sürü (benim muhtemelen) korkunç
  • çoğu insan nasıl PHP şablon için hiçbir ipucu var (benim kitap Smarty önerir <grin>)
  • fonksiyonları Bir ipucu (bilmiyorum) varsa, müthiş

Okuma sayfaları ve kıdemli kodlayıcı ramblings sayfalarında ışığında, belki de dünyanın en paranoyak ve bilinçli acemi hale gelmiştir.

Şimdi benim ilk doğan çocuk kadar sunan, önce secde neden olan ... err, alçakgönüllü çalışma kod yığın, ve sen ve gözyaşı olduğunu yalvarıyorum Ben daha süratle Benim yollardan hata olduğunu öğrenmek, parçalara onu eleştirmek ...

The goals of the following code:

1) kendi unitid sorgu amaçları ve unitname için ekran için dönen, unit_basic_data kendi milliyete dayalı tablodan birimlerinin bir listesini tut kullanıcı.

2) bu formatta her birim için onay kutularını bir form oluşturun

`<tr><td><input type="checkbox" name="Americanunit[]" value="$unitid" />$unitname</td></tr>`

Bu bana dinamik bir ülke, daha sonra seçmek eklemek için izin - Sen "Amerikan" Şu vea manuel $nation olarak ayarlayın ve sonra unit[] eklenir dikkat edeceğiz.

Her kutusunu kontrol 3) için, tabloya bir güncelleştirme sorgusu göndermek scenario_needunits senaryo id oluşan scenid ve birim kimliği unitid, hangi alınır Bir çift birincil anahtar olarak.

4) kullanıcı ve tamamlama mesajın bir onay güncellendi unitids kaba bir listesini Dökümü.

What the code does now: Everything! :-) Except in a rather ugly fashion.

Obvious problems: hiçbir sanitizasyonu (btw, onay kutularını sanitizasyonunun gerektirir mi?), Kullanıcı bir senaryo göndermek için çalışırsa raporlama hata | tabloda zaten ünite çiftini

Further questions to the experts:

  • Is there a cleaner way to build the checkbox strings besides my approach that concatenates SIX variables?
  • Bu bir çiftleşmiş iğrençliktir?
  • Can it be split into multiple files? (if yes, how/where?)
  • Zaten umutsuz muyum?

Kenara yukarıda, ben% 100 başka yorumlara açık & duyuyorum eleştiri. Ben zaman için en nazik hepinize teşekkür ederim, ve sabır. Ve ben SO çok daha güzel görüneceğini nakletmek kod sonraki bit söz veriyorum!

Kod şöyle:

<?php
// designed to dynamically load a country's unit list from the database
// ve allow you to submit the country's units needed for a scenario
error_reporting(E_ALL);
require_once 'login.php';
$db_server = mysql_connect($db_hostname, $db_username, $db_password);

if (!$db_server) die("Unable to connect to MySQL: " . mysql_error());

mysql_select_db($db_database)
or die("Unable to select database: " . mysql_error());

$nation = "American"; // desired nation
echo "$nation is selected.<br />";
$sql = "SELECT unitid,unitname FROM `unit_basic_data` WHERE `forcename`='$nation'"; // grab all current units for $nation
$result = mysql_query($sql);
if (!$result) die ("Database access failed: " . mysql_error());
$rows = mysql_num_rows($result);
echo "Total rows returned is $rows rows."; // display total number of units
$formOpen = '<form method="post" action="scenario-needunits2.php"><input type="hidden" name="submittedCheck" value="yes" /><br />';
$scenario = '<tr><td>Scenario Name: <input type="text" name="scenid" /></td></tr>'; // input target scenario for new data
$checkPre = '<input type="checkbox" name="'; // checkbox string part 1 of 5
$checkName = $nation; // checkbox string part 2 of 5
$checkMid = 'unit[]" value="'; // checkbox string part 3 of 5
$checkPost = '" />'; // checkbox string part 5 of 6
$row = ''; // checkbox string parts (4 of 6) & (6 of 6)
$checkSubmit = '<br /><input type="submit" /></form>'; // submit button & close form
echo "<table><tr><th>Units</th></tr>";
echo "$formOpen";
echo "$scenario";
for ($j = 0 ; $j < $rows ; ++$j) // for statement that returns result row by row
{
	$row = mysql_fetch_row($result);
	echo "<tr>";
	echo "<td>$checkPre$checkName$checkMid$row[0]$checkPost$row[1]</td>"; //brutal, but effective
            echo "</tr>";
}
echo "</table>";
echo "$checkSubmit<br />"; // form closes here

if (isset($_POST['submittedCheck'])) //begin table update code
    {
        $scenid = $_POST['scenid']; // grab scenid
        $americanunit = $_POST['Americanunit']; // grab unit array
        foreach($americanunit as $unitid) // run through array
        {
            $sql = "INSERT INTO scenario_needunits VALUES ('$scenid' , '$unitid');"; // create query string
            $result = mysql_query($sql); // add one new row
            echo "Inserted a $unitid for scenario $scenid<br />"; // report addition
        }
        echo "Complete."; // report update has been successful
    }
?>

4 Cevap

Bahar akla dört şey vardır:

  1. Kod okumak zordur;
  2. Sunum ve işleme mantığı ayrılık yoktur;
  3. HTML bu şekilde çıkışını muhtemelen benim en tercih edilen yöntemdir; ve
  4. Sizin kod SQL enjeksiyon açıktır.

İşte benim öneri.

  1. Işleme (bir "denetleyici" olarak adlandırılır) mantık ve (bir "görünüm" olarak adlandırılır) sunum halinde kod bölmek;
  2. Bu okumak çok daha kolay olabilir gibi mantıklı fonksiyonları ile HTML öğelerini oluşturmak; ve
  3. Her zaman SQL enjeksiyonu ve XSS (cross-site scripting) açıkları karşı savunmak amacıyla şüpheli olarak kullanıcı girişi tedavi; ve
  4. Dahil bir dosyaya ortak kodu koyun.

Örneğin:

config.php

<?php
error_reporting(E_ALL | E_STRICT);
$db_server = mysql_connect($db_hostname, $db_username, $db_password);
if (!$db_server) die("Unable to connect to MySQL: " . mysql_error());
mysql_select_db($db_database) or die("Unable to select database: " . mysql_error());
?>

pagename.php

<?php
require 'config.php';
require 'login.php'; // i assume this checks to see they are logged in?

if (/* user pressed submit */) {
  // do some processing
  if (/* it worked */) {
    $message = 'Success';
  } else {
    $message = 'Failure'; // possibly with explanation why
  }
} else {
  // initialize some values
}

require 'viewname.php';
?>

viewname.php

<form method="post">
<?php echo create_checkbox('name', true /* checked */); ?>
<table>
<?php foreach ($rows as $row): ?>
<tr>
  <td><?php echo create_checkbox(...); ?></td>
</tr>
<?php endforeach; ?>
</table>
</form>

Pagename.php yılında mantığı MVC (Model-View-Controlelr) anlamda bir ham denetleyicisi. MVC ortak bir UI kalıptır. Bazıları "ihtiyaç" bir çerçeve (CodeIgniteger, Kohana, Zend, CakePHP, Symfony, vb) size iddia edecektir ama yok. Onlar çeşitli faydaları var ama yukarıdaki görünümü ve denetleyici bir basit (ve genellikle yeterlidir) bölünür. Görünümü büyük ölçüde sadece kendisine verilen verileri alır bir şablon olmalıdır.

Yukarıda sadece bir görünümü var ama kolayca birden görüşlerini aramak için denetleyicisi şube olabilir.

Template functions can really help on code readability. Örneğin:

function create_checkbox($name, $checked = false) {
  $ch = $checked ? ' checked' : '';
  return <<<END
<input type="checkbox" name="$name" id="$name"$ch>
END;
}

Bu Defa / şablonları denebilecek ve much daha okunabilir olacaktır. Eğer söylemek daha var bir işlevi varsa 2-3 argümanlar yerine seçenekleri bir dizi iletin:

function create_checkbox($options) {
  $checked = $options['checked'] ? ' checked' : '';
  $class = $options['class'] ? ' class="' . $options['class'] . '"' : '';
  $id = $options['name'];
  $name = $options['name'];
  return <<<END
<input type="checkbox"$id$name$class$checked>
END;
}

ile:

<?php echo create_checkbox(array('name' => 'cb1', 'checked' => true, 'class' => 'cbclass')); ?>

Son olarak, never Bu gibi kullanıcı girişini kabul:

$field = $_POST['field'];
$sql = 'INSERT INTO tablename (blah) VALUES ('$field');
mysql_query($sql);

Yani a huge security hole (creating a vulnerability to an attack called "SQL Injection"). Always use mysql_real_escape_string() :

$field = mysql_real_escape_string($_POST['field']);
$sql = 'INSERT INTO tablename (blah) VALUES ('$field');
mysql_query($sql);

XSS önlemek için sık sık kullanıcı girişi üzerinden etiketleri şerit isteyeceksiniz. Bunu yapmak için Th kolay yolu:

$field = filter_var($_POST['field'], FILTER_SANITIZE_STRING);

Bunu yapabileceğiniz farklı filtreler ve doğrulaması vardır. Bkz filter_var() and the types of filters.

Indentation. Yukarıdaki kod okumak için sadece mümkün değildir; birkaç ay içinde bunu imkansız korumak için bulacaksınız.

String slinging. PHP çiftleşmiş dildir. Kullanın, onunla mücadele etmeyin. Eğer zahmetle HTML dizeleri araya zaman size noktayı kaçırıyorsun elle hazırlanmış ettik. Kullanım PHP kendi <?php interpolation ?> tercih. Bir, tek, iyi biçimlendirilmiş bir hiyerarşi var yani girinti ile birleştirin.

Bu gerçek bir çözüm değil, çünkü String escaping., bir dakika için "sterilize" unut. Girişi Sanitising Eğer onlar modeli uygun emin olmak için belirli parametreler için geçerli bir etki alanı özgü fonksiyonu, sen "kötü karakter kurtulmak" için denemek için gönderilen tüm parametreler arasında kesmek gerekir not şeydir. There is no such thing as a bad character, only one you've handled inappropriately.

HTML gibi olmayan bir ham metin bağlam içine ham metni birleştirmek zaman, ister aracılığıyla $html= "<blah>$name</blah>" veya <?php echo ?>, HTML için uygun bir biçime kaçmak gerekir. İşte htmlspecialchars bunu çağırıyor demektir.

Eğer bir SQL deyimi haline ham metni birleştirmek zaman, SQL için uygun bir biçime kaçmak gerekir. Bu tek alıntı sararak önce mysql_real_escape_string çağırıyor demektir. Tek tırnak içeren veya tek tırnak çıkarmadan bütün dizeleri reddetmek anlamına gelmez; adlı bir kişiye O'Reilly, bu kesme çok önemlidir.

(Özellikle SQL için, ayrıca tamamen kaçan endişe, hatta üst düzey bir veri erişim katmanı durur, Parametrelenmiş sorguları düşünmelisiniz.)

Yani görünümü bir parçası gibi görünebilir:

<?php
    // Shortcut to htmlspecialchars for convenience.
    // Would normally put in an include library.
    //
    function h($s) {
        echo(htmlspecialchars($s, ENT_QUOTES));
    }
?>

<?php
    // Get a country's unit list
    //
    $units= mysql_query(
        "SELECT unitid, unitname FROM unit_basic_data ".
        "WHERE forcename='".mysql_real_escape_string($nation)."'"
    );
?>

<form method="post" action="units.php">
    <table>
        <tr>
            <td>
                <label for="f-scenid">Scenario name</label>
            </td>
            <td>
                <input type="text" name="scenid" id="f-scenid" />
            </td>
        </tr>
        <?php while($unit= mysql_fetch_assoc($units)) { ?>
            <tr>
                <td>
                    <label for="unit-<?php h($unit['unitid']) ?>">
                        <?php h($unit['unitname']) ?>
                    </label>
                </td>
                <td>
                    <input type="checkbox"
                        name="<?php h($nation) ?>unit[]"
                        value="<?php h($unit['unitid']) ?>"
                        id="unit-<?php h($unit['unitid']) ?>"
                    />
                </td>
            </tr>
        <?php } ?>
    </table>
    <div>
        <input type="submit" value="Submit" />
        <input type="hidden" name="action" value="insert" />
    </div>
</form>

benim iki sent:

  • PHP (Ben hangi) yeni başlayanlar için korkunç

PHP yeni başlayanlar ya da bir başkası için gayet dildir. Bu web programlama için çok uygundur bulunuyor.

  • PHP korkunç bir ilk dil (Bu benim ilk)

PHP gayet ilk dildir. Çok yerleşik işlevleri vardır, çünkü bir acemi gibi bir kod birkaç satır bir sürü yapabilirsiniz. Bu iyi hissettiriyor, eglence pekiştiriyor. Daha sonra, muhtemelen nesne yönelimli programlama ve birim test hakkında daha ciddi almak için birinden bir Java kitap ödünç almak isteyeceksiniz. Java dünyasında iyi fikirler bir sürü PHP taşıdık edilmiştir.

  • çok ve PHP kod sürü (benim muhtemelen) korkunç

every programlama dilinde çok ve kod çok korkunç. Sevgiler muhtemelen, ama hiç önemli değil. Daha fazla kod yazmak ve okumak daha kod, daha iyi kod alırsınız, bunu çalışmak sağladı. Bu iyi İngilizce yazma öğrenme gibi aynı.

  • çoğu insan nasıl PHP şablon (benim kitap Smarty önerir) hiçbir ipucu var

Smarty aslında oldukça güzel çiftleşmiş kütüphanedir. Ben çok fazla bu odaklanmak olmaz. PHP programlama çiftleşmiş ile ilgisi neredeyse yok.

  • fonksiyonları Bir ipucu (bilmiyorum) varsa, müthiş

Bir seferde bir adım. Fonksiyonlar parçalar halinde bir hesaplama kırarak düşünülen organize bir yoludur. Fonksiyonları ve nesne-scoped değişkenleri içeren nesneler, örgütün daha yüksek bir düzeyde sunmak; ilgili nesneler, daha yüksek bir seviyeye gruplarını organize yollarını tanımlamak tasarım desenleri. Yine, yazmayı öğrenme gibi: tutarlı cümleler içine yapılanma düşünceleri bir şey var; tutarlı paragraflar, bölümler, kitaplar - daha sonra gelir ve bir kerede tüm öğrenemez.

Ana şey kodu veya kendi özensiz kod yazmak ve sadece ona tutmak ', stil duygusu geliştirmek diğer insanların kırmak için korkmak değil başlatmak için kod' diğer insanların çok okuyor. FTW newbz!

Bazı ipuçları

  1. Kullan PDO veritabanına erişmek için (Pro's and con's of using PDO)
  2. Ile tanışın MVC
  3. Ben şahsen Code Complete ve Pragmatik Programcı öneririz bazı books programlama uygulamaları okuyun