views:

210

answers:

5

I'm still new to certain PHP functions. Is there any way I can clean up the following code, because I know all of this is just unnecessary, and it's giving me a headache. Everything after the if statement is the same for each set of code.

    if($class == "2"){if ($posts >= 1){
$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25  WHERE `users`.`friendid`='$friend'";
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());
if (!mysql_query($sql)) {die('Error: ' . mysql_error());} echo "";}
else {$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
mysql_query($sql) or die(mysql_error());
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());}}


if($class == "3"){if ($posts >= 2){
$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());
if (!mysql_query($sql)) {die('Error: ' . mysql_error());} echo "";}
else {$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
mysql_query($sql) or die(mysql_error());
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());}


if($class == "4"){if ($posts >= 3){
$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());
if (!mysql_query($sql)) {die('Error: ' . mysql_error());} echo "";}
else {$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
mysql_query($sql) or die(mysql_error());
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());}}


if($class == "5"){if ($posts >= 4){
$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());
if (!mysql_query($sql)) {die('Error: ' . mysql_error());} echo "";}
else {$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
mysql_query($sql) or die(mysql_error());
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());}}


if($class == "6"){if ($posts >= 5){
$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());
if (!mysql_query($sql)) {die('Error: ' . mysql_error());} echo "";}
else {$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
mysql_query($sql) or die(mysql_error());
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());}}


if($class == "7"){if ($posts >= 6){
$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error());
if (!mysql_query($sql)) {die('Error: ' . mysql_error());} echo "";}
else { $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
mysql_query($sql) or die(mysql_error());
$insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
mysql_query($insert) or die(mysql_error()); }}}}}}
A: 

This is a little easiar on the eyes and easiar to work with, you had some extra } in there that I removed.

I would break some of the repetitive code up into a couple functions at the very least

<?php

if ($class == "2") {
    if ($posts >= 1) {
     $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25  WHERE `users`.`friendid`='$friend'";
     $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
     mysql_query($insert) or die(mysql_error());
     if (!mysql_query($sql)) {
      die('Error: ' . mysql_error());
     }
     echo "";
    } else {
     $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
     mysql_query($sql) or die(mysql_error());
     $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
     mysql_query($insert) or die(mysql_error());
    }
}

if ($class == "3") {
    if ($posts >= 2) {
     $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
     $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
     mysql_query($insert) or die(mysql_error());
     if (!mysql_query($sql)) {
      die('Error: ' . mysql_error());
     }
     echo "";
    } else {
     $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
     mysql_query($sql) or die(mysql_error());
     $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
     mysql_query($insert) or die(mysql_error());
    }
    if ($class == "4") {
     if ($posts >= 3) {
      $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
      $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
      mysql_query($insert) or die(mysql_error());
      if (!mysql_query($sql)) {
       die('Error: ' . mysql_error());
      }
      echo "";
     } else {
      $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
      mysql_query($sql) or die(mysql_error());
      $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
      mysql_query($insert) or die(mysql_error());
     }
    }
    if ($class == "5") {
     if ($posts >= 4) {
      $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
      $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
      mysql_query($insert) or die(mysql_error());
      if (!mysql_query($sql)) {
       die('Error: ' . mysql_error());
      }
      echo "";
     } else {
      $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
      mysql_query($sql) or die(mysql_error());
      $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
      mysql_query($insert) or die(mysql_error());
     }
    }
    if ($class == "6") {
     if ($posts >= 5) {
      $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
      $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
      mysql_query($insert) or die(mysql_error());
      if (!mysql_query($sql)) {
       die('Error: ' . mysql_error());
      }
      echo "";
     } else {
      $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
      mysql_query($sql) or die(mysql_error());
      $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
      mysql_query($insert) or die(mysql_error());
     }
    }
    if ($class == "7") {
     if ($posts >= 6) {
      $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25 WHERE `users`.`friendid`='$friend'";
      $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
      mysql_query($insert) or die(mysql_error());
      if (!mysql_query($sql)) {
       die('Error: ' . mysql_error());
      }
      echo "";
     } else {
      $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
      mysql_query($sql) or die(mysql_error());
      $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
      mysql_query($insert) or die(mysql_error());
     }
    }
}

?>
jasondavis
+1  A: 

If it simple as that - everything after the if statement is the same for each set of code - you can simply create function and call it for each class.

Digging through this code gives me headache too, so I did not do it throughout, but it looks like some major refactoring could be done here.

What I was able to tell at first glance - you do not need all these if statements, because they always fit into pattern:

if ($posts >= ($class - 1)) {
    ...
}
samuil
+1  A: 

I'd extract two functions out of there, insert() and update(). Here's the code for insert(), I leave the other one for you:

/**
 * @param  string $table  Table name to insert $values into
 * @param  array  $values Key is field name, value is field value to insert
 * @return null
 * @throws Exception if query fails
 */
function insert($table, $values) {
    $sql = array();

    foreach ($values as $field => $value) {
        $sql[] = "`$field` = '" . mysql_real_escape_string($value) . "'";
    }

    $sql = "INSERT INTO `$table` SET " . implode(', ', $sql);

    if (! mysql_query($q)) {
        throw new Exception(mysql_error());
    }
}

After this step, use it and see what further code duplication is there and extract it into some other functions.

In the case of update() you'll need a third parameter for the WHERE part.

Ionuț G. Stan
+3  A: 

Didn’t you learn something from your previous question?

if ($class >= 2 && $class <= 7) {
    if ($posts >= ($class - 1)) {
        $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25  WHERE `users`.`friendid`='$friend'";
        $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
        mysql_query($insert) or die(mysql_error());
        if (!mysql_query($sql)) {
            die('Error: ' . mysql_error());
        }
    } else {
        $sql = "UPDATE users SET posts=posts+1,tposts=tposts+1 WHERE `users`.`friendid` = '$friend'";
        mysql_query($sql) or die(mysql_error());
        $insert = "INSERT INTO proofs ( `ID` ,`friendid` ,`bulletinid`)VALUES('', '$friend' , '$binfo');";
        mysql_query($insert) or die(mysql_error());
    }
}
Gumbo
@Joey: But the pattern was almost the same. So you first could have tried it on your own.
Gumbo
+2  A: 

Yikes that hurts my eyes. Here are a few tips.

0 - Write code for humans to read. The most important lesson to learn is that code is for humans to read not computers.

1 - Code style. Pay attention to the curly brackets. As a general rule a { should cause all code between the { and the accompanying } to be indented. 4 spaces is a reasonable amount to indent. This makes the intent of your code easier to determine and therefore easier to spot mistakes.

2 - Validate all input. Never trust anything the user submits. In your sql statements you have the following:

$sql = "UPDATE users SET posts=posts+1,tposts=tposts+1,points=points+25,tpoints=tpoints+25  WHERE `users`.`friendid`='$friend'";`

The problem is the $friend variable. I presume this is being based on user input. The user could enter something nasty like 2; DELETE * FROM * (ok, that SQL may not be correct, but you get the idea). Validate all input and always use SQL parameters.

3 - Use functions. Functions should be used to reduce code duplication. Reducing the number of statements in your code, reduces the potential for errors. Functions should be used to indicate the intent of the code. For example if you have a complicated expression for an if statement you could move the expression to a separate function. For example if (isDateInRange($date, $range)) {}

Finally, spend time reflecting on your code. Ask yourself 'is the intent of the code clear?', 'is there a better way to do that?'. Make sure you understand every single line of your code. The temptation when you're starting out is to have the attitude 'hooray, it works! Let's move on'. Resist this temptation, it will not serve you well in the long run.

Benedict Cohen
$friend = $_SESSION['userid']; located in my config file.
Homework
@Joey. My point was that you should always validate input. Always use SQL parameters, regardless of where the data comes from.
Benedict Cohen