Making my function better

Question

How could I make this code more efficient? Net-beans is saying that I have too many lines in my functions. Also it is saying that where I have my if statement I should introduce a new function. Could some body help me with creating a good function and making my function smaller

function dispalyEvent($weekNr, $week, $year){
    echo "<p>";
    $gendate = new DateTime();
    $gendate->setISODate($year,$week,$weekNr);

    $month = $gendate->format('m');
    $day =  $gendate->format('d');
    $event_query = mysql_query("SELECT * FROM calendar ORDER BY starttime"); 

    while($event = mysql_fetch_array($event_query)) { 
    $startYear = $event['startyear'];
    $startMonth = $event['startmonth'];
    $startDay = $event['startdate'];
    $endYear = $event['endyear'];
    $endMonth = $event['endmonth'];
    $endDay = $event['enddate'];

    $period = new DatePeriod(
            new DateTime($startYear.$startMonth.$startDay),
            new DateInterval('P1D'),
            new DateTime($endYear.$endMonth.$endDay +1)
    );

    $currentDate = $year."-".$month."-".$day;

        foreach ($period as $savedDate) {   

            if ($currentDate == $savedDate->format('Y-m-d')){
                buildEvent($event['ad'], $event['starttime'], $event['title'], $event['endtime'], $event['location'], $event['address'], $event['price'], $event['description']);
            }    
            if ($event['Approved'] == "Approved"){
                    buildEvent($event['ad'], $event['starttime'], $event['title'], $event['endtime'], $event['location'], $event['address'], $event['price'], $event['description']);
            }

        }

    }
    echo "</p>";
}
?>

One way of improving it is to stop using the mysql extension, since it's officially deprecated. You should instead use either MySQLi or PDO. — Tularis
– Tularis, Commented Mar 28, 2014 at 23:23
If you want someone to review your code you should post on Code Review — user1864610
– user1864610, Commented Mar 31, 2014 at 4:31

Tularis · Accepted Answer · 2014-03-28 23:36:17Z

2

Aside from switching to MySQLi or PDO for safety and support reasons; you can also clear most of your unused code.

For example: you don't actually have to create new variables for every array-value.

Here's what I would make of it:

<?php
function dispalyEvent($weekNr, $week, $year){
    echo "<p>";
    $currentDate = new DateTime()->setISODate($year,$week,$weekNr)->format('Y-m-d');

    $event_query = mysql_query("SELECT * FROM calendar ORDER BY starttime");         
    while($event = mysql_fetch_array($event_query)) { 
        $period = new DatePeriod(
            new DateTime($event['startyear'].$event['startmonth'].$event['startday']),
            new DateInterval('P1D'),
            new DateTime($event['endyear'].$event['endmonth'].(event['$endday']+1))
        );

        foreach ($period as $savedDate) {   
            if ($currentDate == $savedDate->format('Y-m-d') || $event['Approved'] == "Approved"){
                buildEvent($event['ad'], $event['starttime'], $event['title'], $event['endtime'], $event['location'], $event['address'], $event['price'], $event['description']);
            }
        }
    }
    echo "</p>";
}
?>

answered Mar 28, 2014 at 23:36

Tularis

1,5061 gold badge8 silver badges17 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Pieter de Vries Over a year ago

I get an error on $currentDate = new DateTime()->setISODate($year,$week,$weekNr)->format('Y-m-d');

martskins · Accepted Answer · 2014-03-28 23:35:29Z

1

You shouldn't take NetBeans warning that seriously, not that kind at least. Most of them are configurable and you can change them. Those are useful when working with a team, if you want to "force" your team to follow certain rules you set those rules to help you verify the code.

Other than what Tularis said I don't see what you can do. Please remember that a good code is not always short, code that is easily understandable is as good as it gets.

answered Mar 28, 2014 at 23:35

martskins

2,9604 gold badges29 silver badges53 bronze badges

Collectives™ on Stack Overflow

Making my function better

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related