4
\$\begingroup\$

The goal of this code is to create a drop-down menu where there are 14 options on the list. Those 14 options are the dates that an RSS feed was last updated. The URLs for the feed contain the query &issue=2014/10/14, but the trick is that it's set so that if a date is entered when the feed wasn't updated, it returns the most recent one instead of an error.

This code finds today's date, goes back one day, builds the URL for the feed for that day, loads that feed and checks if the publication date matches the date in that URL (thereby confirming that the feed was updated that day), and if it's a valid day it builds the option value and adds it to the drop-down list. Then it does that again until there are 14 options.

It also checks whether a day is a weekend first, to skip all the weekend days because I know this feed won't be updated on weekends.

The problem is that if there are big gaps between valid dates, it takes too long to execute and I get a FastCGI timeout error. I'm not interested in lengthening the FastCGI timeout length. What I've done is set the time limit to 2 seconds, which is enough to go through about a month's worth of days and get as many valid days as there are in there.

I feel like this is a really sloppy way to do it but I can't come up with anything better. Hopefully someone with more experience has a better solution!

<?php 
error_reporting(0);
$timer = set_time_limit(2);
echo ("<select id='select1' align=center width=10 onchange='javascript:location.href=this.value;'>");
echo ("<option>-- select past date --</option>");
$i=1;
$j=0;
$pickdate = date("m-d-Y"); //today's date
while ($j <= 13) {
    $pickdate = mktime(0, 0, 0, date("m")  , date("d")-$i, date("Y")); //subtract from today's date
    $new_pickdate = date('D. M. j, Y',$pickdate ); //put date in desired format for dropdown
    $postdate = date('Y/m/j',$pickdate); //put date in desired format for url 
    $xml=("http://[IPremoved]/NewsLetter/postfeed?type=1&issue=".$postdate); 
    $onchangeurl=("http://enews-archive.php?".$postdate);
    $dayofweek = date('N',$pickdate ); //pull day of week
    if ($dayofweek < 6) { //check for weekdays 
        $xmlDoc = new DOMDocument(); //load xml to test if empty
        $xmlDoc->load($xml); //load xml to test if empty
        $channel=$xmlDoc->getElementsByTagName('channel')->item(0); //load xml to test if empty
        $pubdate = $channel->getElementsByTagName('PostDate')->item(0)->childNodes->item(0)->nodeValue; //load xml to test if empty
        $pubdatestamp = strtotime($pubdate);
        $checkdate = date('Y/m/j',$pubdatestamp); //put date in desired format to check
        if ($postdate == $checkdate)  //check if dates match
            {
                echo ('<option value='.$onchangeurl.'>'.$new_pickdate.'</option>'); //if weekday & xml not empty print it
            $j++; //add to counter if it's a valid date
        } //end match check 
    } //end weekday check
    $i++; //increase subtraction from date
} //end while
echo ("</select>");
?>
\$\endgroup\$
5
  • \$\begingroup\$ "it returns the most recent one instead of an error". Do you mean the most recent feed compared to today, or the most recent feed compared to the date you give? Have you considered caching the results (assuming they don't backdate feeds)? \$\endgroup\$ Commented Oct 14, 2014 at 18:55
  • \$\begingroup\$ There isn't much you can do to optimize this code. You have more of an architectural problem instead, save the last 14 known dates to a file. If the file was updated today, just spit out those URLs. If the file was updated before today, use the last known date and regenerate the URLs one by one until you reach today and re-save those dates to the text file. \$\endgroup\$ Commented Oct 14, 2014 at 19:16
  • \$\begingroup\$ Barry: It returns the most recent compared to today. I don't know anything about caching... Can you recommend something to read to get me started on that? \$\endgroup\$ Commented Oct 14, 2014 at 19:50
  • \$\begingroup\$ Greg: I like that idea. Unfortunately I don't really have the slightest idea how to begin doing that, in the middle of a drop-down. But I'll see what I can come up with. Thanks! \$\endgroup\$ Commented Oct 14, 2014 at 19:53
  • \$\begingroup\$ @LauraM: you need to separate the code that generates the data from the code that generates the dropdown. \$\endgroup\$ Commented Oct 14, 2014 at 20:03

1 Answer 1

Reset to default
3
\$\begingroup\$

Review

Bearing in mind that this code was posted nearly 11 years ago, perhaps much has been learned about PHP, RSS feeds and similar topics. Is the code still used today?

As Greg alluded to, the available feeds could be fetched once daily in a scheduled task/cron job and stored locally - e.g. in a database, cache etc. instead of fetching the latest RSS feeds available when generating the list of options. That way the code to add the list of options does not need to depend on the external API - it can simply query the local database/cache.

Suggestions

$timer seems unused

The second line is :

$timer = set_time_limit(2);

set_time_limit() returns a boolean1, so $timer would either be true or false. Additionally it does not appear to be utilized anywhere, so the assignment to $timer can be removed.

$pickdate can be generated in a simpler fashion

Instead of using mktime() to generate the previous days, one could use strtotime(). Instead of this:

$pickdate = mktime(0, 0, 0, date("m") , date("d")-$i, date("Y")); //subtract from today's date

It can simply be:

$pickdate = strtotime("-i days");

initial assignment of $pickdate appears unnecessary

The initial assignment of $pickdate outside the loop - i.e.

$pickdate = date("m-d-Y"); //today's date

can be removed because the value is overwritten at the start of the while loop.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.