CODEWITHSUNDEEP
php
Karem
Karem

Reputation: 18103

PHP: Making cleaner & shorter code

I have this:

<?php
if($_GET['wE'] && is_numeric($_GET['wE'])){
$we = mysql_real_escape_string($_GET['wE']);
$query_find_we = "SELECT id FROM users_wall WHERE id = '$we' AND uID = '$showU[id]'";
$query_find_we = mysql_query($query_find_we)or die(mysql_error());
$grab_wall_comment = (mysql_num_rows($query_find_we) == 1) ? "window.location.hash = '#comment$we';" : "alert('Vägginlägg kunde ej hittas.');";
?>
<script>
$(function() {
<?php echo $grab_wall_comment; ?>
});
</script>
<?php
}elseif($_GET['sE'] && is_numeric($_GET['sE'])){
$se = mysql_real_escape_string($_GET['sE']);
$query_find_se = "SELECT id FROM users_statuslog WHERE id = '$se' AND uID = '$showU[id]'";
$query_find_se = mysql_query($query_find_se)or die(mysql_error());
$grab_status_comment = (mysql_num_rows($query_find_se) == 1) ? "window.location.hash = '#comment$se';" : "alert('Status kunde ej hittas.');";
?>
<script>
$(function() {
<?php echo $grab_status_comment; ?>
});
</script>
<?php
    }
?>

checks if any $_GET['we'] or $_GET['se'], and if it exists. and runs the <script>.

Is there a way to make this shorter? I tried myself alittle with making a boolean, but maybe you could short it even more? Any ideas, as I would like to make cleaner coding in the future..

Upvotes: 0

Views: 344

Answers (2)

Your Common Sense
Your Common Sense

Reputation: 157839

Don't you see it yourself?

There are obviously 2 identical parts of code.
You have to just make variable parts of this code into PHP variables. That's all.
And of course there should be some db API function in use.

<?
if (!empty($_GET['wE'])) {
  $id = $_GET['wE']
  $table = "users_wall";
  $alert = "Vagginlagg kunde ej hittas.";
} elseif (!empty($_GET['sE'])) {
  $id = $_GET['sE'];
  $table = "users_statuslog";
  $alert = "Status kunde ej hittas.";
}
$query = "SELECT count(id) FROM `$table` HERE id = %d AND uID = %d";
$count = db::getOne($query,$id,$showU['id']);

//Separate your main PHP logic from presentation as much as possible.
//leave only necessary operators.
?>
<script>
$(function() {
<? if($count): ?>window.location.hash = '#comment<?=$id?>';
<? else: ?>alert('<?=$alert?>');
<? endif ?>
});
</script>

getone() function is similar to getarr() I mentioned in this answer but returns a scalar value instead of array.

Upvotes: 1

pleasedontbelong
pleasedontbelong

Reputation: 20102

<?php
    $param = (isset($_GET['wE']) && is_numeric($_GET['wE']))?"wE":((isset($_GET['sE']) && is_numeric($_GET['sE']))?"sE":false);
    if($param){
        echo showScript($param);
    } else {
        //TODO: do something if there's no 'wE' nor 'sE' maybe?
    }

    function showScript($param){
        if($param == "wE"){
            $table = "users_wall";
            $alert = "Vägginlägg kunde ej hittas.";
        } else {
            $table = "users_statuslog";
            $alert = "Status kunde ej hittas.";
        }
        $e = mysql_real_escape_string($_GET[$param]);
        $query_find = "SELECT id FROM ".$table." WHERE id = '".$e."' AND uID = '$showU[id]'";
        $query_find = mysql_query($query_find)or die(mysql_error());
        $grab_status_comment = (mysql_num_rows($query_find) == 1) ? "window.location.hash = '#comment$e';" : "alert('$alert');";
        return '<script>$(function() {'.$grab_wall_comment.'});</script>';
    }
    ?>

i haven't tested it... but that's the main idea :P

good luck, hope this helps

Upvotes: 0

Related Questions