Help me clean this code up please

[jbearnolimits]

I've got a bit of a mess lol. My goal is to have 3 pages. One for tasks over due, one for current due tasks, and one for future due tasks. What I have done is created a database with columns for each task (up to 15) and named them Task1, Task2, and so on. These columns hold the instructions for the user to complete their task for the day. I have another column called TaskDueDate to hold the due dates. When a new task is added a new row is created for each task (row 1 has the Task1 column filled in and the TaskDueDate filled in, then row 2 has the Task2 column filled in and the TaskDueDate filled in and so on).

Now, here's what I have done. On the Future Due Tasks page I have created multiple sql queries. One query for each task due. In the query I limit it to only tasks that are not null, only tasks in certain lists (which is another column in the table), and only tasks that are due on future dates.

This works to get the future tasks. BUT I am not able to order the results properly. I end up with the first few rows being all task 1 items even though some task 2's have earlier dates they are further down in the list. I'll show you the code so you can see what I mean:

$sql = "SELECT UserID, FirstName, LastName, Phone, HomeAddress, ListName, Task1, TaskDueDate FROM Tasks WHERE ListName = 'FSBO' AND TaskDueDate > '$TaskDueDate' AND Task1 IS NOT NULL OR ListName = 'Expired' AND TaskDueDate > '$TaskDueDate' AND Task1 IS NOT NULL OR ListName = 'FSBO In Escrow' AND TaskDueDate > '$TaskDueDate' AND Task1 IS NOT NULL OR ListName = 'FSBO Fell Out Of Escrow' AND TaskDueDate > '$TaskDueDate' AND Task1 IS NOT NULL";
$result = $conn->query($sql);
if ($result->num_rows > 0) {
 // output data of each row
 while($row = $result->fetch_assoc()) {
echo "<div class=\"OuterDatabaseRowType2\">
<div class=\"InnerDatabaseRowType2ProfilID\"><ul class=\"second\"><li class=\"second\"><a href=\"../CompleteFutureTask.php/?UserID=" . $row["UserID"] . "&TaskDueDate=" . $row["TaskDueDate"] . "&ListName=" . $row["ListName"]. "&Phone=" . $row["Phone"] . "\">Complete Task</a></li></ul></div>
<div class=\"InnerDatabaseRowType2Name\">" . $row["FirstName"]. " " . $row["LastName"]. "</div>
<div class=\"InnerDatabaseRowType2Phone\">" . $row["Phone"]. "</div>
<div class=\"InnerDatabaseRowType2HomeAddress\"><textarea name=\"Task1\" id=\"Task1\" rows=\"5\" cols=\"45\">";?><?php echo str_replace(array('#HomeAddress#', '#FirstName#', '#LastName#'), array($row["HomeAddress"], $row["FirstName"], $row["LastName"]), $row["Task1"]);?><?php echo "</textarea> </div>
<div class=\"InnerDatabaseRowType2CreationDate\">" . $row["ListName"]. "</div>
          <div class=\"InnerDatabaseRowType2Green\"></div>
</div>";
 }
}

$sql = "SELECT UserID, FirstName, LastName, Phone, HomeAddress, ListName, Task2, TaskDueDate FROM Tasks WHERE ListName = 'FSBO' AND TaskDueDate > '$TaskDueDate' AND Task2 IS NOT NULL OR ListName = 'Expired' AND TaskDueDate > '$TaskDueDate' AND Task2 IS NOT NULL OR ListName = 'FSBO In Escrow' AND TaskDueDate > '$TaskDueDate' AND Task2 IS NOT NULL OR ListName = 'FSBO Fell Out Of Escrow' AND TaskDueDate > '$TaskDueDate' AND Task2 IS NOT NULL";
$result = $conn->query($sql);

if ($result->num_rows > 0) {
 // output data of each row
 while($row = $result->fetch_assoc()) {
echo "<div class=\"OuterDatabaseRowType2\">
<div class=\"InnerDatabaseRowType2ProfilID\"><ul class=\"second\"><li class=\"second\"><a href=\"../CompleteFutureTask.php/?UserID=" . $row["UserID"] . "&TaskDueDate=" . $row["TaskDueDate"] . "&ListName=" . $row["ListName"]. "&Phone=" . $row["Phone"] . "\">Complete Task</a></li></ul></div>
<div class=\"InnerDatabaseRowType2Name\">" . $row["FirstName"]. " " . $row["LastName"]. "</div>
<div class=\"InnerDatabaseRowType2Phone\">" . $row["Phone"]. "</div>
<div class=\"InnerDatabaseRowType2HomeAddress\"><textarea name=\"Task2\" id=\"Task2\" rows=\"5\" cols=\"45\">";?><?php echo str_replace(array('#HomeAddress#', '#FirstName#', '#LastName#'), array($row["HomeAddress"], $row["FirstName"], $row["LastName"]), $row["Task2"]);?><?php echo "</textarea> </div>
<div class=\"InnerDatabaseRowType2CreationDate\">" . $row["ListName"]. "</div>
          <div class=\"InnerDatabaseRowType2Green\"></div>
</div>";
 }
}

As you can see the code is far too messy. I feel that there must be a way to sort it out better than this. I could use some help figuring out how to clean it up so that I can use only 1 sql query if possible to get each task due in the future and order them all by date without having to display the results in batches of task1's and then task2's and so on.

[lucy24]

<tangent>
What's the reason for the recurring

;?><?php 

closing and immediately re-opening php?

Are you allowed to use parentheses? That steady stream of AND and OR makes me uneasy.
</tangent>

[jbearnolimits]

It makes me uneasy too lol. Which is why I am asking about how I can do this properly.

And yeah, I see the point about the PHP opening and closing. This is a bit of a Frankenstein monster. Lots of copy and paste as well as being a rush job. I needed something that works at least half way so I didn't worry about keeping it clean. Kinda like sweeping the dirt under the rug and now that company has left it's time to actually clean up.

[jbearnolimits]

You will be happy to know that after stepping back and looking at it again I found the solution. I discovered I didn't need all the extra columns and could put all the tasks into one task column and the dates into one date column. Then I got to work on cleaning up the code on the future tasks page and it is not looking like this:

$sql = "SELECT UserID, FirstName, LastName, Phone, HomeAddress, ListName, Task1, Task1DueDate FROM Tasks WHERE Task1DueDate > '$Task1DueDate' ORDER BY Task1DueDate ASC";
$result = $conn->query($sql);
if ($result->num_rows > 0) {
 // output data of each row
 while($row = $result->fetch_assoc()) {
echo "<div class=\"OuterDatabaseRowType2\">
<div class=\"InnerDatabaseRowType2ProfilID\"><ul class=\"second\"><li class=\"second\"><a href=\"../CompleteFutureTask.php/?UserID=" . $row["UserID"] . "&Task1DueDate=" . $row["Task1DueDate"] . "&ListName=" . $row["ListName"]. "&Phone=" . $row["Phone"] . "\">Complete Task</a></li></ul></div>
<div class=\"InnerDatabaseRowType2Name\">" . $row["FirstName"]. " " . $row["LastName"]. "</div>
<div class=\"InnerDatabaseRowType2Phone\">" . $row["Phone"]. "</div>
<div class=\"InnerDatabaseRowType2HomeAddress\"><textarea name=\"Task1\" id=\"Task1\" rows=\"5\" cols=\"45\">";?><?php echo str_replace(array('#HomeAddress#', '#FirstName#', '#LastName#'), array($row["HomeAddress"], $row["FirstName"], $row["LastName"]), $row["Task1"]);?><?php echo "</textarea> </div>
<div class=\"InnerDatabaseRowType2CreationDate\">" . $row["ListName"]. "</div>
                   <div class=\"InnerDatabaseRowType2Green\"></div>
</div>";
 }


} else {
echo "No More Results";
}

I'm going to fix the opening and closing of the php in a moment. But I wanted to show how it was all able to be condensed into one sql query.

[jbearnolimits]

On another note I have one more question related to this. As you can see there is a "Complete Task" page linked to each row. On that row I have the following:

$sql = "DELETE FROM Tasks WHERE Task1DueDate = '$Task1DueDate' AND UserID = '$UserID' AND Phone = '$Phone' AND ListName != 'Basic Keep In Touch'";
$sql = "UPDATE Tasks SET Task1DueDate = '$NewDate' WHERE UserID = $UserID AND Task1DueDate = '$Task1DueDate' AND ListName = 'Basic Keep In Touch'";

The problem is that the row isn't deleted or updated like this. But if I take one sql query out (like the bottom one) then it functions. It just doesn't BOTH function together. I thought you could do 2 sql queries, was I wrong?

[lucy24]

Sit tight. We need someone who actually speaks sql. (You will have figured out that I don't; I just look at general structure.)

[londrum]

I know it doesn't make any real difference, but if it was me I would change all the

echo “

to

echo ‘

because then you can change all the \” to just “
makes it easier to look at

[robzilla]

The problem is that the row isn't deleted or updated like this. But if I take one sql query out (like the bottom one) then it functions. It just doesn't BOTH function together. I thought you could do 2 sql queries, was I wrong?

I don't see you executing those queries in your code snippet. Try to include all the relevant information in your snippets.

After all, if you write:

$sql = "DELETE FROM Tasks WHERE Task1DueDate = '$Task1DueDate' AND UserID = '$UserID' AND Phone = '$Phone' AND ListName != 'Basic Keep In Touch'";
$sql = "UPDATE Tasks SET Task1DueDate = '$NewDate' WHERE UserID = $UserID AND Task1DueDate = '$Task1DueDate' AND ListName = 'Basic Keep In Touch'";

$result = $conn->query($sql);

Then of course only the UPDATE query will be executed because you've overwritten the value of $sql containing the DELETE before sending the query to the database.

If you write:

$sql = "DELETE FROM Tasks WHERE Task1DueDate = '$Task1DueDate' AND UserID = '$UserID' AND Phone = '$Phone' AND ListName != 'Basic Keep In Touch'";
$result = $conn->query($sql);

$sql = "UPDATE Tasks SET Task1DueDate = '$NewDate' WHERE UserID = $UserID AND Task1DueDate = '$Task1DueDate' AND ListName = 'Basic Keep In Touch'";
$result = $conn->query($sql);

Then both should be executed.

[jbearnolimits]

Ah, yes. Thanks, I did not realize it would overwrite.

[phranque]

I did not realize it would overwrite

just to clarify - this is a php issue, not a database/sql issue.