5
\$\begingroup\$

I made a simple calendar in PHP 7.0, could anyone help with simplification? Any advice appreciated. It works, but I totally believe it can be improved. I am a beginner.

<?php

  ini_set('display_errors', 1);
  ini_set('display_startup_errors', 1);
  error_reporting(E_ALL);

  $chosen_year = filter_input(INPUT_GET, 'chosen_year', FILTER_SANITIZE_SPECIAL_CHARS);
  $chosen_month = filter_input(INPUT_GET, 'selected_month', FILTER_SANITIZE_SPECIAL_CHARS);
  $do_update_button = filter_input(INPUT_GET, 'month_year', FILTER_SANITIZE_SPECIAL_CHARS);

  if (isset($do_update_button)) {

    $number_of_days = date("t", mktime(0, 0, 0, $chosen_month, 1, $chosen_year));

    $english_order = date("l", mktime(0, 0, 0, $chosen_month, 1, $chosen_year));

  } else {

    $number_of_days = date("t", mktime(0, 0, 0, date('n'), 1, date('Y')));      

    $english_order = date("l", mktime(0, 0, 0, date('n') , 1, date('Y')));    

  }

  $blank_cell = '&nbsp;';

  $array_of_days = array($blank_cell,"1","2","3","4","5","6","7","8","9","10",
    "11","12","13","14","15","16","17","18","19","20",
    "21","22","23","24","25","26","27","28","29","30",
    "31");

  // search for the first day of month     

  if($english_order == "Monday") {
    $a = 1;   
  }

  else if($english_order == "Tuesday") {
    array_unshift ($array_of_days, $blank_cell);
    $a = 2;   
  }

  else if($english_order == "Wednesday") {
    array_unshift ($array_of_days, $blank_cell, $blank_cell);
    $a = 3;   
  }

  else if($english_order == "Thursday") {
    array_unshift ($array_of_days, $blank_cell, $blank_cell, $blank_cell);
    $a = 4;     
  }

  else if($english_order == "Friday") {
    array_unshift ($array_of_days, $blank_cell, $blank_cell, $blank_cell, $blank_cell);
    $a = 5;   
  }

  else if($english_order == "Saturday") {
    array_unshift ($array_of_days, $blank_cell, $blank_cell, $blank_cell, $blank_cell, $blank_cell);
    $a = 6;
  }

  else if($english_order == "Sunday") {
    array_unshift ($array_of_days, $blank_cell, $blank_cell, $blank_cell, $blank_cell, $blank_cell, $blank_cell);
    $a = 7;  
  }

  $lastDay = count($array_of_days);

?>

<form action = "callender.php" method = "GET"> 
<table border = "1"> 

<!-- choose year --> 

<th colspan = "2">
<select name = "chosen_year">
<option value = "<?php echo date('Y'); ?>" selected = "selected"><?php echo date('Y') ?> </option>  

<?php

  $chosen_year = filter_input(INPUT_GET, 'chosen_year', FILTER_SANITIZE_SPECIAL_CHARS);

  function getYear() {

    $years = "";

    for ($year = 2030; $year >= 2000; $year--) {

      $years .= $year . " ";  

    }

    return $years;

   }

   $years = explode (" ", getYear());

   foreach ($years as $number => $chose_year) {

?>

  <option value="<?php echo $chose_year; ?>"<?php if(isset($chosen_year) and ($chosen_year) == $chose_year) { ?> selected = "selected"<?php } else if(date('Y') == $number)  { ?> selected ='selected'<?php } ?>> <?php echo $chose_year; ?></option>

<?php

  }

?>

  </select>
  </th>

  <!-- select month -->

  <th colspan = "3">
  <select name = "selected_month"> 

<?php

    $chosen_month = filter_input(INPUT_GET, 'selected_month', FILTER_SANITIZE_SPECIAL_CHARS);

    $months = array(1 => "January", "February", "March", "April", "May",
       "June", "July", "August", "September", "October", "November",
       "December");

     foreach($months as $number2 => $chose_month) {

?>

 <option value="<?php echo $number2;?>"<?php if(isset($chosen_month) and ($chosen_month)== $number2)  { ?> selected = "selected"<?php } ?>>  

 <?php echo $chose_month; ?></option>        

<?php } ?>

</select>    
   </th>    
   <th colspan = "2">
     <input type = "submit" value = "update" name="month_year" />    
   </th>

   <tr><td>Mon</td> <td>Tue</td><td>Wed</td><td>Thu</td><td>Fri</td><td>Sat</td><td>Sun</td></tr>

<?php

  $i = 1;

  while ($i < $number_of_days + $a) {

    if ((($i - 1  ) % 7 == 0)) {  

      $DayToBrake = $i;

      echo "\n<tr>";

    }

else if ($i != 1) echo "</td>\n";

   echo "<td>";      

   if (($chosen_year == date('Y')) and ($chosen_month == date('n')) and date('j') == $array_of_days[$i]) {

     echo '<strong style="color:red">' .$array_of_days[$i]. '</strong>';

   } else {

    echo  $array_of_days[$i];

   }

  if (($i % 7 == 0) || ($i == $lastDay)) {

    $DayToBrake = $i;

    echo "</td></tr>";

  }

  $i++;    

  }

  echo "</td></tr>\n";

?>

  <tr>

    <td colspan = "5">

<?php $m = date('n'); {echo '<strong>'." Is"." ".$months[$m]." ". date('Y').'</strong>';} ?>

    </td>

    <td colspan = "2">

    </td>

 </tr>

 </table>

 </form>
\$\endgroup\$
5
\$\begingroup\$

There are any number of things to get into here since, as you say you are a beginner. This code looks a lot like what you may have seen in PHP code examples on the web 10+ years ago, when PHP was used mostly as a web scripting language to add dynamic functionality to otherwise static web pages. So let me first introduce you to what I feel is the best source out there in how to get started working with PHP - PHP: The Right Way. There is a lot of information here, but it is worthwhile to digest it over time, as this is a better source of information than probably 99% of PHP code examples out there.


First, you should strive to separate your PHP logic from your HTML output. Right now you have the classic "spaghetti code" which is very hard to maintain over time. Do your very best to write all the information you need into PHP variables, such that when you get to rendering your HTML (ideally in a different file from your main code), all you are doing is simple variable insertion and perhaps some looping in your HTML template.


I would strongly encourage you to familiarize yourself with PHP's DateTime and related classes. They are much more powerful in working with date manipulation than the old procedural date() functions.

This can eliminate large portions of your script where you are doing things like manually building date arrays and finding first days of months and such.

// instantiate DateTime based on current time
$datetime = new DateTime('first day of this month');
// change to first day of next month
$datetime = $datetime->modify('+1 month');
// set up iterator of every month start date for full date range between 2000 and 2030
$startDate = new DateTime('2000-01-01 00:00:00');
$endDate = new DateTime('2030-12-31 23:59:59');
$monthlyInterval = new DateInterval('1m');
$weeklyInterval = new DateInterval('1w');
$dailyInterval = new DateInterval('1d');
$monthlyPeriods = new DatePeriod($startDate, $monthlyInterval, $endDate);
foreach($monthlyPeriods as $firstDayOfMonth) {
    // make calendar based on DateTime stored in $firstDayOfMonth
    // for example let's say this calendar shows Sunday-Saturday and
    // includes days that may be in previous/next month
    // Note: DateTime considers Monday as first day of week for relative
    // this/week calculations so this jives well with your Mon-Sun calendar
    $firstDayOnCalendar = clone($firstDayOfMonth)
        ->modify('Monday this week');
    $lastDayOnCalendar = clone($firstDayOfMonth)
        ->modify('last day of this month')
        ->modify('Sunday this week');

    // build weeks
    $weekPeriods = new DatePeriod($firstDayOnCalendar, $weeklyInterval, $lastDayOnCalendar);
    foreach($weekPeriods as $firstSundayOfWeek) {
        $days = new DatePeriod($firstSundayOfWeek, $dailyInterval, 7);
        foreach ($days as $day) {
            // build day cell on calendar using $day
            echo $day->format('Y-m-d');
        }
    }
}

This also automatically handles all concerns around daylight savings times, leap years, etc. and you can also incorporate timezone considerations using DateTimeZone

Tying to manually work with dates the way you are doing is a bit of a fool's errand.


Get in the habit of using exact comparisons (=== and !==) instead of loose comparisons (== and !=). These will go a long way in making your code less fragile to unexpected truthy/falsey behavior that can lead to bugs. You should only use loose comparisons in cases where you truly have a need to match loosely. You might find that these cases are really few and far between.


Your variable names could certainly be more meaningful. What does $english_order convey? What does $do_update_button mean? getYear() does not get a year, it gets a range of years, right?


Don't mix function definitions randomly in the middle of procedural sections of code like you are doing with getYear()


You are doing some input sanitization (good!), But you really are not validating input in any sense. I could send totally unexpected values for months/years to this script and cause it to break. Never trust user input - validate every single bit of it.


You have way too much vertical white space (i.e. blank lines) in the HTML output part of your script. This makes your code hard to read.

\$\endgroup\$
  • \$\begingroup\$ Thank you very much. But I don't know how to combine the output: prnt.sc/esmj6h with HTML now. Could you please point me to the right direction? Thanks in advance. \$\endgroup\$ – Pavel Apr 5 '17 at 7:13
  • \$\begingroup\$ @Pavel I don't know what your link is supposed to be. \$\endgroup\$ – Mike Brant Apr 5 '17 at 18:26
  • \$\begingroup\$ @MikeBrant I believe it's an image of the web page generated by your code without any kind of formatting. \$\endgroup\$ – LinuxSecurityFreak Apr 7 '17 at 5:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.