I have a huge function filled with nested blocks

Could someone help me on how to eliminate some nested blocks or improve this code? I am concerned this will slow down my site dramatically.

```
function dispalyEvent($weekNr, $week, $year){
echo "";

$gendate = new DateTime();
$gendate->setISODate($year,$week,$weekNr);
$event_query = mysql_query("SELECT * FROM calendar ORDER BY starttime");

//Go through all event in the database
while($event = mysql_fetch_array($event_query)) {

//Create a range for starting date and ending date
$date1 = new DateTime($event['startyear'].$event['startmonth'].$event['startdate']);
$date2 = new DateTime($event['endyear'].$event['endmonth'].$event['enddate']);
$date2->modify('+1 day');

$period = new DatePeriod($date1, new DateInterval('P1D'), $date2);

$title = $event['title'];
$name = $event['name'];

$recur_query = mysql_query("SELECT * FROM recur WHERE title = '$title' AND name = '$name'");
$recur = mysql_fetch_array($recur_query);

$recurring = $recur['type'];

//Find day of starting recurring event and ending day
if (!$recurring == "None"){
$starttime = explode("/",$recur['startdate']);
$startdate = new DateTime();
$startdate->setDate($starttime[2], $starttime[0], $starttime[0]);

$endtime = explode("/",$recur['enddate']);
$enddate = new DateTime();
$enddate->setDate($endtime[2], $endtime[0], $endtime[0]);

}
else {
$startdate = new DateTime();
$enddate = new DateTime();
}

//Put the dates in integer to find if it is out of range
$displaydate = intval($gendate->format("Ymd"));
$startdate = intval($startdate->format("Ymd"));
$enddate = intval($enddate->format("Ymd"));
settype($displaydate, "integer");
settype($startdate, "integer");

Ok, the following review may seem blunt or harsh, but please, try to keep in mind that this is in order to help. I'm not trying to hurt or mock anyone, but in order for code-review to be as effective as it ought to be, I'll have to be brutal.

If you haven't read it already, the help-section asks you post working code. bug-riddled code isn't subject to review yet, it has to be debugged first.

It is possible you aren't aware of it, and that you may think your code works, when really it doesn't. Well, not as you expect it to, at least. I know it feels banal and tedious, closely looking at the operator precedence table doesn't do any harm. Quite the opposite, in fact. You'll soon find out why Both David Harkness and myself mention potenial bugs or unexpected behaviour with expressions like:

if (!$recurring == "None")
//and
if ($displaydate > $startdate and !$displaydate < $enddate)

And as a last point in this foreword to what is already a hefty answer, I would like to strongly suggest you change your php.ini settings for the error_reporting and set display_errors to true, one, or stdout, depending on your PHP version.

The error_reporting's default value is likely to be E_ALL & ~E_NOTICE & ~E_STRICT & ~E_DEPRECATED, while debugging, it's best to set it to E_ALL | E_STRICT, or call error_reporting(-1); in your script.

As I have done before, I'll walk through your code line by line, offering advice and the reasoning behind my criticism. At the end, I'll add an example of code you could end up with if you decide to take my recommendations to heart.

Update: I did not add a code example as there are simply too many unknowns to deal with, and that any example would basically end up being a total re-write of your code, which isn't my job, and is of little educational use to you. Instead, just to make it 100% clear, however blunt or harsh this answer may seem here's a meta-post on why I consider it necessary for code-review to be tough

Now, without further ado, let's get too it:

function dispalyEvent($weekNr, $week, $year){

Yes, I have some criticisms about the very first line of code you posted already. Ok, a function displayEvent, that expects 3 arguments. All three have to do with time. But if you need variables that tell you something about time, why not ask of the user (caller) to pass a DateTime class from the off?

function displayEvent(DateTime $date)
{

Now this tells the user of your code that he's expected to pass a DateTime instance as an argument. It reduces the number of arguments from 3 to 1, and allows for type-hints. As we'll see in a second, this also saves you the bother of creating the DateTime instances inside the function. The advantage of that is that, if the caller already has a DateTime instance, he can simply pass that object, and not call methods to get the year, week and weekNr values, which are only being used to re-construct the same DateTime instance all over.

Onwards:

echo "";

Don't echo in a function. A function returns. The caller of your function may then choose to echo the return value, or may store it somewhere else. Having a function echo something puts the user of your code in a tight spot: calling this function means he can't set the headers, can't use this function to retrieve data and present it in a way he wants to. Just create a variable $outString = '';, and return that at the end.

$gendate = new DateTime();
    $gendate->setISODate($year,$week,$weekNr);

As I said before: this code can be made redundant simply by changing the function's signature to expect a DateTime instance from the off

$event_query = mysql_query("SELECT * FROM calendar ORDER BY starttime");

    //Go through all event in the database
    while($event = mysql_fetch_array($event_query)) {

Please, please, please stop using the deprecated mysql_ extension. Switch to PDO or mysqli_ instead. Henceforth I'll be using PDO.

And as a rule of thumb, or even personal mantra: Avoid SELECT * queries whenever you can. Select what you need, and how you need it. You haven't done that last bit at all, judging by the next snippet of code:

//Create a range for starting date and ending date
    $date1 = new DateTime($event['startyear'].$event['startmonth'].$event['startdate']);
    $date2 = new DateTime($event['endyear'].$event['endmonth'].$event['enddate']);
    $date2->modify('+1 day');

Why not select these dates like so:

SELECT CONCAT_WS('-', startyear, startmonth, startdate) AS date1

That way, you'll be able to write:

$date1 = new DateTime($event['date1']);

That's just, I think you'll agree, a hell of a lot cleaner. Anyway, back to the code:

$period = new DatePeriod($date1, new DateInterval('P1D'), $date2);

     $title = $event['title'];
     $name = $event['name'];

Why bother assigning individual variables, you have an associative array, what's wrong with that? An assoc

StackExchange Code Review Q#45801, answer score: 10