|
Posted by Steve on 04/16/07 03:32
it's sloppy. plus you assume that everything in $_POST is a radio 'checked'
input. better to define, for you and your other developers, what it is for
which you are looking. further, what is all this for='r1' non-sense in your
html?
clean it up. all of your php tags that are not inline echos should be to the
left of the screen so that you can easily tell where you are breaking into
and out of php code...i don't care how nested the php code is, always put
the <?php and ?> at the extreme left hand side!
i'd name my radio buttons in such a way that php will make them into arrays.
that way you can have them in $_POST with a bunch of other inputs yet work
with them seperately.
finally, in your html, all you have to do is echo 'checked' rather than
checked='checked'. there is no need to use SetChecked...especially since you
are already echoing out 'checked' (when necissary) in the same input tag!
anyway, i'd overlook almost all of that if you'd at least use good coding
standards of practice.
"Samuel van Laere" <webkluns@yahoo.com> wrote in message
news:4622756e$0$321$e4fe514c@news.xs4all.nl...
| When looking at the very basic "script" below,
| are there things that could be done in an better way more effective?
| Is there anything I do wrong when it comes to security or checking values
| used in _POST?
| Also I wonder why PHP's highlight_file is putting each string on a
seperate
| line, any idea's?
|
| The full code:
| <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
| "http://www.w3.org/TR/html4/strict.dtd">
| <html lang="nl">
| <head>
| <link rel="stylesheet" type="text/css" media="screen,projection"
| href="forms.css">
| <base href="http://www.webkluns.nl/">
| </head>
| <body>
| <?php
| $rb = $_REQUEST['r1'];
| if ($rb)
| {
| $status = 'Je hebt ' . ($rb == 'Ja' ? 'ja ' : 'nee ') . 'gekozen.';
| } else {
| $status = 'Je hebt nog geen keuze gemaakt.';
| }
|
| function SetChecked($rb,$val){
| if (isset($_POST[$rb])){
| $checked = ($_POST[$rb]==$val) ? "checked=\"checked\"" : "";
| }
| return $checked;
| }
| ?>
| <fieldset>
| <legend>Broncode weergeven?</legend>
| <form action = "<?php echo $_SERVER['PHP_SELF'];?>" method = "post">
| <p>
| <input class="radio" <?php echo SetChecked('r1','Ja');?>type="radio"
| id="r1" name="r1" value="Ja" <?php ($r1 == 'Ja' ? 'checked' : '') ?>>
| <label for="r1">Ja<span>laat mij de broncode zien</span></label>
| </p>
| <p>
| <input class="radio" <?php echo SetChecked('r1','Nee');?>type="radio"
| id="r2" name="r1" value="Nee" <?php ($r1 == 'Nee' ? 'checked' : '') ?>>
| <label for="r2">Nee<span>dat boeit me niet</span></label>
| </p>
| <p>
| <input class="submit" type="submit" name="submit" value="Bevestig">
| </p>
| <p><strong><?php echo $status; ?></strong></p>
| </form>
| </fieldset>
| </legend>
| <?php if ($rb == 'Ja'){
| echo "<div id=\"code\">";
| echo "<h3>De broncode:</h3>";
| echo "<code>";
| $deze_pagina = basename ($_SERVER['PHP_SELF']);
| $str = highlight_file ($deze_pagina);
| echo "</code>";
| echo "</div>";}?>
| </body>
| </html>
|
| The online example:
| http://www.webkluns.nl/test.php
|
| Thanks in advance for your comments or critique.
|
| Cheers,
| Sam
|
|
Navigation:
[Reply to this message]
|